All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Scally <djrscally@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org
Cc: Mark Gross <mgross@linux.intel.com>
Subject: Re: [PATCH v2 4/8] platform/x86: intel_skl_int3472: Use ACPI GPIO resource directly
Date: Mon, 21 Jun 2021 23:37:14 +0100	[thread overview]
Message-ID: <a9493e7a-5cde-085a-145b-56d246dd0bcc@gmail.com> (raw)
In-Reply-To: <20210618125516.53510-4-andriy.shevchenko@linux.intel.com>

On 18/06/2021 13:55, Andy Shevchenko wrote:
> When we call acpi_gpio_get_io_resource(), the output will be
> the pointer to the ACPI GPIO resource. Use it directly instead of
> dereferencing the generic resource.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


This is much better, thanks.


Reviewed-by: Daniel Scally <djrscally@gmail.com>

Tested-by: Daniel Scally <djrscally@gmail.com>


> ---
> v2: new patch
>  .../intel_skl_int3472_clk_and_regulator.c     |  7 ++---
>  .../intel-int3472/intel_skl_int3472_common.h  |  2 +-
>  .../intel_skl_int3472_discrete.c              | 28 +++++++++----------
>  3 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> index ceee860e2c07..49ea1e86c193 100644
> --- a/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> @@ -131,10 +131,10 @@ int skl_int3472_register_clock(struct int3472_discrete_device *int3472)
>  }
>  
>  int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> -				   struct acpi_resource *ares)
> +				   struct acpi_resource_gpio *agpio)
>  {
> -	char *path = ares->data.gpio.resource_source.string_ptr;
>  	const struct int3472_sensor_config *sensor_config;
> +	char *path = agpio->resource_source.string_ptr;
>  	struct regulator_consumer_supply supply_map;
>  	struct regulator_init_data init_data = { };
>  	struct regulator_config cfg = { };
> @@ -168,8 +168,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
>  						int3472->regulator.supply_name,
>  						&int3472_gpio_regulator_ops);
>  
> -	int3472->regulator.gpio = acpi_get_and_request_gpiod(path,
> -							     ares->data.gpio.pin_table[0],
> +	int3472->regulator.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0],
>  							     "int3472,regulator");
>  	if (IS_ERR(int3472->regulator.gpio)) {
>  		dev_err(int3472->dev, "Failed to get regulator GPIO line\n");
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> index 6fdf78584219..765e01ec1604 100644
> --- a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> @@ -113,6 +113,6 @@ union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev,
>  int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb);
>  int skl_int3472_register_clock(struct int3472_discrete_device *int3472);
>  int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> -				   struct acpi_resource *ares);
> +				   struct acpi_resource_gpio *agpio);
>  
>  #endif
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> index 48a00a1f4fb6..fd681d2a73fe 100644
> --- a/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> @@ -103,11 +103,11 @@ skl_int3472_get_sensor_module_config(struct int3472_discrete_device *int3472)
>  }
>  
>  static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int3472,
> -					  struct acpi_resource *ares,
> +					  struct acpi_resource_gpio *agpio,
>  					  const char *func, u32 polarity)
>  {
> -	char *path = ares->data.gpio.resource_source.string_ptr;
>  	const struct int3472_sensor_config *sensor_config;
> +	char *path = agpio->resource_source.string_ptr;
>  	struct gpiod_lookup *table_entry;
>  	struct acpi_device *adev;
>  	acpi_handle handle;
> @@ -145,7 +145,7 @@ static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int347
>  
>  	table_entry = &int3472->gpios.table[int3472->n_sensor_gpios];
>  	table_entry->key = acpi_dev_name(adev);
> -	table_entry->chip_hwnum = ares->data.gpio.pin_table[0];
> +	table_entry->chip_hwnum = agpio->pin_table[0];
>  	table_entry->con_id = func;
>  	table_entry->idx = 0;
>  	table_entry->flags = polarity;
> @@ -156,23 +156,22 @@ static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int347
>  }
>  
>  static int skl_int3472_map_gpio_to_clk(struct int3472_discrete_device *int3472,
> -				       struct acpi_resource *ares, u8 type)
> +				       struct acpi_resource_gpio *agpio, u8 type)
>  {
> -	char *path = ares->data.gpio.resource_source.string_ptr;
> +	char *path = agpio->resource_source.string_ptr;
> +	u16 pin = agpio->pin_table[0];
>  	struct gpio_desc *gpio;
>  
>  	switch (type) {
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
> -		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
> -						  "int3472,clk-enable");
> +		gpio = acpi_get_and_request_gpiod(path, pin, "int3472,clk-enable");
>  		if (IS_ERR(gpio))
>  			return (PTR_ERR(gpio));
>  
>  		int3472->clock.ena_gpio = gpio;
>  		break;
>  	case INT3472_GPIO_TYPE_PRIVACY_LED:
> -		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
> -						  "int3472,privacy-led");
> +		gpio = acpi_get_and_request_gpiod(path, pin, "int3472,privacy-led");
>  		if (IS_ERR(gpio))
>  			return (PTR_ERR(gpio));
>  
> @@ -242,7 +241,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  
>  	if (!obj) {
>  		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
> -			 ares->data.gpio.pin_table[0]);
> +			 agpio->pin_table[0]);
>  		return 1;
>  	}
>  
> @@ -250,15 +249,14 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  
>  	switch (type) {
>  	case INT3472_GPIO_TYPE_RESET:
> -		ret = skl_int3472_map_gpio_to_sensor(int3472, ares, "reset",
> +		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, "reset",
>  						     GPIO_ACTIVE_LOW);
>  		if (ret)
>  			err_msg = "Failed to map reset pin to sensor\n";
>  
>  		break;
>  	case INT3472_GPIO_TYPE_POWERDOWN:
> -		ret = skl_int3472_map_gpio_to_sensor(int3472, ares,
> -						     "powerdown",
> +		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, "powerdown",
>  						     GPIO_ACTIVE_LOW);
>  		if (ret)
>  			err_msg = "Failed to map powerdown pin to sensor\n";
> @@ -266,13 +264,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  		break;
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
>  	case INT3472_GPIO_TYPE_PRIVACY_LED:
> -		ret = skl_int3472_map_gpio_to_clk(int3472, ares, type);
> +		ret = skl_int3472_map_gpio_to_clk(int3472, agpio, type);
>  		if (ret)
>  			err_msg = "Failed to map GPIO to clock\n";
>  
>  		break;
>  	case INT3472_GPIO_TYPE_POWER_ENABLE:
> -		ret = skl_int3472_register_regulator(int3472, ares);
> +		ret = skl_int3472_register_regulator(int3472, agpio);
>  		if (ret)
>  			err_msg = "Failed to map regulator to sensor\n";
>  

  reply	other threads:[~2021-06-21 22:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-18 12:55 [PATCH v2 1/8] platform/x86: Remove "default n" entries Andy Shevchenko
2021-06-18 12:55 ` [PATCH v2 2/8] platform/x86: intel_skl_int3472: Free ACPI device resources after use Andy Shevchenko
2021-06-21 22:32   ` Daniel Scally
2021-06-18 12:55 ` [PATCH v2 3/8] platform/x86: intel_skl_int3472: Fix dependencies (drop CLKDEV_LOOKUP) Andy Shevchenko
2021-06-18 12:55 ` [PATCH v2 4/8] platform/x86: intel_skl_int3472: Use ACPI GPIO resource directly Andy Shevchenko
2021-06-21 22:37   ` Daniel Scally [this message]
2021-06-18 12:55 ` [PATCH v2 5/8] platform/x86: intel_skl_int3472: Provide skl_int3472_unregister_regulator() Andy Shevchenko
2021-06-21 22:40   ` Daniel Scally
2021-06-18 12:55 ` [PATCH v2 6/8] platform/x86: intel_skl_int3472: Provide skl_int3472_unregister_clock() Andy Shevchenko
2021-06-18 12:55 ` [PATCH v2 7/8] platform/x86: intel_skl_int3472: Move to intel/ subfolder Andy Shevchenko
2021-06-18 12:55 ` [PATCH v2 8/8] platform/x86: intel_cht_int33fe: Move to its own subfolder Andy Shevchenko
2021-06-22  9:02 ` [PATCH v2 1/8] platform/x86: Remove "default n" entries Andy Shevchenko
2021-06-22  9:03   ` Hans de Goede
2021-06-22  9:42 ` Hans de Goede
2021-06-22  9:46   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9493e7a-5cde-085a-145b-56d246dd0bcc@gmail.com \
    --to=djrscally@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=hdegoede@redhat.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mgross@linux.intel.com \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.