linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: tegra186: Add ACPI support
@ 2021-06-17 10:05 Akhil R
  2021-06-17 14:54 ` Linus Walleij
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Akhil R @ 2021-06-17 10:05 UTC (permalink / raw)
  To: akhilrajeev, Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter
  Cc: linux-gpio, linux-tegra, linux-kernel, Mikko Perttunen,
	Laxman Dewangan, Suresh Mangipudi, Krishna Yarlagadda

From: Akhil Rajeev <akhilrajeev@nvidia.com>

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Signed-off-by: Akhil Rajeev <akhilrajeev@nvidia.com>
---

 drivers/gpio/gpio-tegra186.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1bd9e44..c8051be 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -5,6 +5,7 @@
  * Author: Thierry Reding <treding@nvidia.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -620,13 +621,18 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
+	if (has_acpi_companion(&pdev->dev)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+	} else {
+		gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
+		gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
+	}
 
-	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
 	if (IS_ERR(gpio->secure))
 		return PTR_ERR(gpio->secure);
 
-	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
 	if (IS_ERR(gpio->base))
 		return PTR_ERR(gpio->base);
 
@@ -690,11 +696,15 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
-	gpio->gpio.of_node = pdev->dev.of_node;
-	gpio->gpio.of_gpio_n_cells = 2;
-	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	if (!has_acpi_companion(&pdev->dev)) {
+		gpio->gpio.of_node = pdev->dev.of_node;
+		gpio->gpio.of_gpio_n_cells = 2;
+		gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+		gpio->intc.name = pdev->dev.of_node->name;
+	} else {
+		gpio->intc.name = gpio->soc->name;
+	}
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -918,10 +928,21 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+#endif
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = ACPI_PTR(tegra186_gpio_acpi_match),
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] gpio: tegra186: Add ACPI support
  2021-06-17 10:05 [PATCH] gpio: tegra186: Add ACPI support Akhil R
@ 2021-06-17 14:54 ` Linus Walleij
  2021-06-17 15:21 ` Andy Shevchenko
  2021-06-18 19:39 ` [PATCH] " kernel test robot
  2 siblings, 0 replies; 19+ messages in thread
From: Linus Walleij @ 2021-06-17 14:54 UTC (permalink / raw)
  To: Akhil R, Andy Shevchenko
  Cc: Bartosz Golaszewski, Thierry Reding, Jonathan Hunter,
	open list:GPIO SUBSYSTEM, linux-tegra, linux-kernel,
	Mikko Perttunen, Laxman Dewangan, Suresh Mangipudi,
	Krishna Yarlagadda

Hi Akhil,

thanks for your patch!

On Thu, Jun 17, 2021 at 12:05 PM Akhil R <akhilrajeev@nvidia.com> wrote:

> From: Akhil Rajeev <akhilrajeev@nvidia.com>
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil Rajeev <akhilrajeev@nvidia.com>

Please include ACPI GPIO maintainer Andy Shevchenko on subsequent
posts, he is looking after ACPI handling in the GPIO subsystem and
always provide excellent reviews. (Added on To:)

It looks OK to my untrained eye, but I don't know ACPI details
and expected behaviours as well as Andy.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] gpio: tegra186: Add ACPI support
  2021-06-17 10:05 [PATCH] gpio: tegra186: Add ACPI support Akhil R
  2021-06-17 14:54 ` Linus Walleij
@ 2021-06-17 15:21 ` Andy Shevchenko
  2021-06-18 13:39   ` [PATCH v2] " Akhil R
  2021-06-18 19:39 ` [PATCH] " kernel test robot
  2 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-06-17 15:21 UTC (permalink / raw)
  To: Akhil R
  Cc: Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter, open list:GPIO SUBSYSTEM, linux-tegra,
	Linux Kernel Mailing List, Mikko Perttunen, Laxman Dewangan,
	Suresh Mangipudi, Krishna Yarlagadda

On Thu, Jun 17, 2021 at 1:18 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> From: Akhil Rajeev <akhilrajeev@nvidia.com>
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.

...

> +#include <linux/acpi.h>

It probably should be property.h, see below.

...

> +       if (has_acpi_companion(&pdev->dev)) {
> +               gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> +               gpio->base = devm_platform_ioremap_resource(pdev, 1);
> +       } else {
> +               gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> +               gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
> +       }

General comment here.
Can't we rather try named resources first and fallback to indexed
ones? (Or other way around)

In this case you don't need to check for ACPI at all.

...

> +               .acpi_match_table = ACPI_PTR(tegra186_gpio_acpi_match),

You can drop ACPI_PTR() along with ugly ifdeffery.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-17 15:21 ` Andy Shevchenko
@ 2021-06-18 13:39   ` Akhil R
  2021-06-18 14:31     ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Akhil R @ 2021-06-18 13:39 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: akhilrajeev, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

From: Akhil Rajeev <akhilrajeev@nvidia.com>

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Signed-off-by: Akhil Rajeev <akhilrajeev@nvidia.com>
---
v2 changes:
	* fallback to find resource by index if name is not found to support ACPI.
	* removed #ifdef and ACPI_PTR.
	* Apparently, acpi.h is required to support changes @@ -690,11 +697,15 

 drivers/gpio/gpio-tegra186.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1bd9e44..fa1295a 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -5,6 +5,7 @@
  * Author: Thierry Reding <treding@nvidia.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -620,15 +621,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
 
 	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
-	if (IS_ERR(gpio->secure))
-		return PTR_ERR(gpio->secure);
-
 	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
-	if (IS_ERR(gpio->base))
-		return PTR_ERR(gpio->base);
+
+	if (IS_ERR(gpio->secure) || IS_ERR(gpio->base)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+
+		if (IS_ERR(gpio->secure))
+			return PTR_ERR(gpio->secure);
+
+		if (IS_ERR(gpio->base))
+			return PTR_ERR(gpio->base);
+	}
 
 	err = platform_irq_count(pdev);
 	if (err < 0)
@@ -690,11 +697,15 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
-	gpio->gpio.of_node = pdev->dev.of_node;
-	gpio->gpio.of_gpio_n_cells = 2;
-	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	if (!has_acpi_companion(&pdev->dev)) {
+		gpio->gpio.of_node = pdev->dev.of_node;
+		gpio->gpio.of_gpio_n_cells = 2;
+		gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+		gpio->intc.name = pdev->dev.of_node->name;
+	} else {
+		gpio->intc.name = gpio->soc->name;
+	}
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -918,10 +929,19 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = tegra186_gpio_acpi_match,
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-18 13:39   ` [PATCH v2] " Akhil R
@ 2021-06-18 14:31     ` Andy Shevchenko
  2021-06-21 15:08       ` Akhil R
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-06-18 14:31 UTC (permalink / raw)
  To: Akhil R
  Cc: Bartosz Golaszewski, Jon Hunter, Krishna Yarlagadda,
	Laxman Dewangan, Linus Walleij, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List, linux-tegra, Mikko Perttunen,
	Suresh Mangipudi, Thierry Reding

On Fri, Jun 18, 2021 at 4:39 PM Akhil R <akhilrajeev@nvidia.com> wrote:

Thanks for update, my comments below.

> From: Akhil Rajeev <akhilrajeev@nvidia.com>

You need to fix your Git configuration so you won't have this header
inside the commit message.

> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.

...

> +#include <linux/acpi.h>

property.h ? (see below)

...

> -       gpio->soc = of_device_get_match_data(&pdev->dev);
> +       gpio->soc = device_get_match_data(&pdev->dev);
>
>         gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> -       if (IS_ERR(gpio->secure))
> -               return PTR_ERR(gpio->secure);
> -
>         gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
> -       if (IS_ERR(gpio->base))
> -               return PTR_ERR(gpio->base);

> +       if (IS_ERR(gpio->secure) || IS_ERR(gpio->base)) {
> +               gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> +               gpio->base = devm_platform_ioremap_resource(pdev, 1);
> +
> +               if (IS_ERR(gpio->secure))
> +                       return PTR_ERR(gpio->secure);
> +
> +               if (IS_ERR(gpio->base))
> +                       return PTR_ERR(gpio->base);
> +       }


What about doing like

      gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
      if (IS_ERR(gpio->secure))
              gpio->secure = devm_platform_ioremap_resource(pdev, 0);
      if (IS_ERR(gpio->secure))
              return PTR_ERR(gpio->secure);

and similar for gpio->base?

...

> -       gpio->gpio.of_node = pdev->dev.of_node;
> -       gpio->gpio.of_gpio_n_cells = 2;
> -       gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
>
> -       gpio->intc.name = pdev->dev.of_node->name;

> +       if (!has_acpi_companion(&pdev->dev)) {
> +               gpio->gpio.of_node = pdev->dev.of_node;
> +               gpio->gpio.of_gpio_n_cells = 2;
> +               gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
> +               gpio->intc.name = pdev->dev.of_node->name;
> +       } else {
> +               gpio->intc.name = gpio->soc->name;
> +       }

Wouldn't the following be enough?

-       gpio->intc.name = pdev->dev.of_node->name;
+       gpio->intc.name = devm_kasprintf(&pdev->dev, "%pfw",
dev_fwnode(&pdev->dev));
+       if (!gpio->intc.name)
+               return -ENOMEM;

Note, all above are questions and you know better which direction to
take. In either way, please test and look at the result.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] gpio: tegra186: Add ACPI support
  2021-06-17 10:05 [PATCH] gpio: tegra186: Add ACPI support Akhil R
  2021-06-17 14:54 ` Linus Walleij
  2021-06-17 15:21 ` Andy Shevchenko
@ 2021-06-18 19:39 ` kernel test robot
  2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-06-18 19:39 UTC (permalink / raw)
  To: Akhil R, Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter
  Cc: kbuild-all, linux-gpio, linux-tegra, linux-kernel,
	Mikko Perttunen, Laxman Dewangan

[-- Attachment #1: Type: text/plain, Size: 1825 bytes --]

Hi Akhil,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v5.13-rc6 next-20210618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Akhil-R/gpio-tegra186-Add-ACPI-support/20210617-180601
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-s022-20210618 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/b8c186168bab74f0c9b85d7231fb09f562e10242
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Akhil-R/gpio-tegra186-Add-ACPI-support/20210617-180601
        git checkout b8c186168bab74f0c9b85d7231fb09f562e10242
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/gpio/gpio-tegra186: struct acpi_device_id is 32 bytes.  The last of 4 is:
0x4e 0x56 0x44 0x41 0x30 0x34 0x30 0x38 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
>> FATAL: modpost: drivers/gpio/gpio-tegra186: struct acpi_device_id is not terminated with a NULL entry!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46814 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-18 14:31     ` Andy Shevchenko
@ 2021-06-21 15:08       ` Akhil R
  2021-06-29 15:17         ` Akhil R
  2021-06-30 16:14         ` Andy Shevchenko
  0 siblings, 2 replies; 19+ messages in thread
From: Akhil R @ 2021-06-21 15:08 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: akhilrajeev, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding


Thanks Andy for the suggestions. Few thoughts I have below.

>What about doing like

>      gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
>      if (IS_ERR(gpio->secure))
>              gpio->secure = devm_platform_ioremap_resource(pdev, 0);
>      if (IS_ERR(gpio->secure))
>              return PTR_ERR(gpio->secure);
>
>and similar for gpio->base?

Wouldn't this cause a redundant check if it had already succeeded in getting
the resource by name? Also, could it happen that if the device tree is
incorrect, then one of the resource is fetched by name and other by the index,
which I guess, would mess things up. Just my random thoughts, not sure if it
is valid enough.

>Wouldn't the following be enough?
>
>-       gpio->intc.name = pdev->dev.of_node->name;
>+       gpio->intc.name = devm_kasprintf(&pdev->dev, "%pfw",
>dev_fwnode(&pdev->dev));
>+       if (!gpio->intc.name)
>+

How about this way? I feel it would be right to add the OF functions conditionally.

+   if (pdev->dev.of_node) {
+       gpio->gpio.of_node = pdev->dev.of_node;
+       gpio->gpio.of_gpio_n_cells = 2;
+       gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+   }

+   gpio->intc.name = gpio->soc->name;

--
Best Regards,
Akhil

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-21 15:08       ` Akhil R
@ 2021-06-29 15:17         ` Akhil R
  2021-06-30 16:14         ` Andy Shevchenko
  1 sibling, 0 replies; 19+ messages in thread
From: Akhil R @ 2021-06-29 15:17 UTC (permalink / raw)
  To: akhilrajeev
  Cc: andy.shevchenko, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

Hi Andy,

Please let me know if you have any inputs regarding the changes.

--
Best Regards,
Akhil


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-21 15:08       ` Akhil R
  2021-06-29 15:17         ` Akhil R
@ 2021-06-30 16:14         ` Andy Shevchenko
  2021-06-30 17:47           ` Akhil R
  1 sibling, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-06-30 16:14 UTC (permalink / raw)
  To: Akhil R
  Cc: Bartosz Golaszewski, Jon Hunter, Krishna Yarlagadda,
	Laxman Dewangan, Linus Walleij, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List, linux-tegra, Mikko Perttunen,
	Suresh Mangipudi, Thierry Reding

On Mon, Jun 21, 2021 at 6:07 PM Akhil R <akhilrajeev@nvidia.com> wrote:

> >What about doing like
>
> >      gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> >      if (IS_ERR(gpio->secure))
> >              gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> >      if (IS_ERR(gpio->secure))
> >              return PTR_ERR(gpio->secure);
> >
> >and similar for gpio->base?
>
> Wouldn't this cause a redundant check if it had already succeeded in getting
> the resource by name? Also, could it happen that if the device tree is
> incorrect, then one of the resource is fetched by name and other by the index,
> which I guess, would mess things up. Just my random thoughts, not sure if it
> is valid enough.
>
> >Wouldn't the following be enough?
> >
> >-       gpio->intc.name = pdev->dev.of_node->name;
> >+       gpio->intc.name = devm_kasprintf(&pdev->dev, "%pfw",
> >dev_fwnode(&pdev->dev));
> >+       if (!gpio->intc.name)
> >+
>
> How about this way? I feel it would be right to add the OF functions conditionally.

Looks okay, although I have a question here.

> +   if (pdev->dev.of_node) {

Do we really need this check at all? If the OF-node is NULL then it
doesn't matter if other fields are filled or not, correct?

What you need is #ifdef CONFIG_OF_GPIO (IIRC the name correctly).

> +       gpio->gpio.of_node = pdev->dev.of_node;
> +       gpio->gpio.of_gpio_n_cells = 2;
> +       gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
> +   }
>
> +   gpio->intc.name = gpio->soc->name;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] gpio: tegra186: Add ACPI support
  2021-06-30 16:14         ` Andy Shevchenko
@ 2021-06-30 17:47           ` Akhil R
  2021-06-30 18:17             ` [PATCH v3] " Akhil R
  0 siblings, 1 reply; 19+ messages in thread
From: Akhil R @ 2021-06-30 17:47 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: akhilrajeev, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

> > >What about doing like
> >
> >      gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> > >      if (IS_ERR(gpio->secure))
> > >              gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> > >      if (IS_ERR(gpio->secure))
> > >              return PTR_ERR(gpio->secure);
> > >
> > >and similar for gpio->base?
> >
> > Wouldn't this cause a redundant check if it had already succeeded in getting
> > the resource by name? Also, could it happen that if the device tree is
> > incorrect, then one of the resource is fetched by name and other by the index,
> > which I guess, would mess things up. Just my random thoughts, not sure if it
> > is valid enough.
> >
> > >Wouldn't the following be enough?
> > >
> > >-       gpio->intc.name = pdev->dev.of_node->name;
> > >+       gpio->intc.name = devm_kasprintf(&pdev->dev, "%pfw",
> > >dev_fwnode(&pdev->dev));
> > >+       if (!gpio->intc.name)
> > >+
> >
> > How about this way? I feel it would be right to add the OF functions conditionally.
> 
> Looks okay, although I have a question here.
> 
> +   if (pdev->dev.of_node) {
> 
> Do we really need this check at all? If the OF-node is NULL then it
> doesn't matter if other fields are filled or not, correct?
> 
> What you need is #ifdef CONFIG_OF_GPIO (IIRC the name correctly).
> 
> > +       gpio->gpio.of_node = pdev->dev.of_node;
> > +       gpio->gpio.of_gpio_n_cells = 2;
> > +       gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
> > +   }
> >
> > +   gpio->intc.name = gpio->soc->name;

Okay. It makes sense. Thanks Andy. I would make the changes and send out an updated patch.

--
Best Regards,
Akhil

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3] gpio: tegra186: Add ACPI support
  2021-06-30 17:47           ` Akhil R
@ 2021-06-30 18:17             ` Akhil R
  2021-06-30 18:23               ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Akhil R @ 2021-06-30 18:17 UTC (permalink / raw)
  To: akhilrajeev
  Cc: andy.shevchenko, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Change-Id: Id8e892f989e4ccc935b87aa0d84b10a3d1efd8f9
Signed-off-by: Akhil Rajeev <akhilrajeev@nvidia.com>
---
 drivers/gpio/gpio-tegra186.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1bd9e44..3b553c2 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
 
 	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
-	if (IS_ERR(gpio->secure))
-		return PTR_ERR(gpio->secure);
-
 	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
-	if (IS_ERR(gpio->base))
-		return PTR_ERR(gpio->base);
+
+	if (IS_ERR(gpio->secure) || IS_ERR(gpio->base)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+
+		if (IS_ERR(gpio->secure))
+			return PTR_ERR(gpio->secure);
+
+		if (IS_ERR(gpio->base))
+			return PTR_ERR(gpio->base);
+	}
 
 	err = platform_irq_count(pdev);
 	if (err < 0)
@@ -690,11 +696,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
+#if defined(CONFIG_OF_GPIO)
 	gpio->gpio.of_node = pdev->dev.of_node;
 	gpio->gpio.of_gpio_n_cells = 2;
 	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+#endif /* CONFIG_OF_GPIO */
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	gpio->intc.name = dev_name(&pdev->dev);
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -918,10 +926,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = tegra186_gpio_acpi_match,
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] gpio: tegra186: Add ACPI support
  2021-06-30 18:17             ` [PATCH v3] " Akhil R
@ 2021-06-30 18:23               ` Andy Shevchenko
  2021-07-01  5:01                 ` [PATCH v4] " Akhil R
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-06-30 18:23 UTC (permalink / raw)
  To: Akhil R
  Cc: Bartosz Golaszewski, Jon Hunter, Krishna Yarlagadda,
	Laxman Dewangan, Linus Walleij, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List, linux-tegra, Mikko Perttunen,
	Suresh Mangipudi, Thierry Reding

On Wed, Jun 30, 2021 at 9:17 PM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.

Thanks for an update, my comments below.
After addressing, feel free to add
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Change-Id: Id8e892f989e4ccc935b87aa0d84b10a3d1efd8f9

This is not for upstream.

> Signed-off-by: Akhil Rajeev <akhilrajeev@nvidia.com>

...

> +static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
> +       { .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
> +       { .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
> +       { .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
> +       { .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },

> +       {},

Comma is not needed for terminator lines.

> +};
> +MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4] gpio: tegra186: Add ACPI support
  2021-06-30 18:23               ` Andy Shevchenko
@ 2021-07-01  5:01                 ` Akhil R
  2021-07-01  5:53                   ` Jon Hunter
  0 siblings, 1 reply; 19+ messages in thread
From: Akhil R @ 2021-07-01  5:01 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: akhilrajeev, bgolaszewski, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/gpio/gpio-tegra186.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1bd9e44..e0ba8cd 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
 
 	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
-	if (IS_ERR(gpio->secure))
-		return PTR_ERR(gpio->secure);
-
 	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
-	if (IS_ERR(gpio->base))
-		return PTR_ERR(gpio->base);
+
+	if (IS_ERR(gpio->secure) || IS_ERR(gpio->base)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+
+		if (IS_ERR(gpio->secure))
+			return PTR_ERR(gpio->secure);
+
+		if (IS_ERR(gpio->base))
+			return PTR_ERR(gpio->base);
+	}
 
 	err = platform_irq_count(pdev);
 	if (err < 0)
@@ -690,11 +696,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
+#if defined(CONFIG_OF_GPIO)
 	gpio->gpio.of_node = pdev->dev.of_node;
 	gpio->gpio.of_gpio_n_cells = 2;
 	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+#endif /* CONFIG_OF_GPIO */
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	gpio->intc.name = dev_name(&pdev->dev);
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -918,10 +926,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = tegra186_gpio_acpi_match,
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v4] gpio: tegra186: Add ACPI support
  2021-07-01  5:01                 ` [PATCH v4] " Akhil R
@ 2021-07-01  5:53                   ` Jon Hunter
  2021-07-01  9:00                     ` [PATCH v5] " Akhil R
  0 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2021-07-01  5:53 UTC (permalink / raw)
  To: Akhil R, andy.shevchenko
  Cc: bgolaszewski, kyarlagadda, ldewangan, linus.walleij, linux-gpio,
	linux-kernel, linux-tegra, mperttunen, smangipudi,
	thierry.reding


On 01/07/2021 06:01, Akhil R wrote:
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/gpio/gpio-tegra186.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index 1bd9e44..e0ba8cd 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>  	if (!gpio)
>  		return -ENOMEM;
>  
> -	gpio->soc = of_device_get_match_data(&pdev->dev);
> +	gpio->soc = device_get_match_data(&pdev->dev);
>  
>  	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> -	if (IS_ERR(gpio->secure))
> -		return PTR_ERR(gpio->secure);
> -
>  	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
> -	if (IS_ERR(gpio->base))
> -		return PTR_ERR(gpio->base);
> +
> +	if (IS_ERR(gpio->secure) || IS_ERR(gpio->base)) {


The OR here seems a bit odd, my preference would be how Andy suggested
initially ...

    gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
    if (IS_ERR(gpio->secure)) {
        gpio->secure = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(gpio->secure))
            return PTR_ERR(gpio->secure)
    }

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v5] gpio: tegra186: Add ACPI support
  2021-07-01  5:53                   ` Jon Hunter
@ 2021-07-01  9:00                     ` Akhil R
  2021-07-08 13:08                       ` Jon Hunter
  2021-07-16  8:31                       ` Bartosz Golaszewski
  0 siblings, 2 replies; 19+ messages in thread
From: Akhil R @ 2021-07-01  9:00 UTC (permalink / raw)
  To: jonathanh
  Cc: akhilrajeev, andy.shevchenko, bgolaszewski, kyarlagadda,
	ldewangan, linus.walleij, linux-gpio, linux-kernel, linux-tegra,
	mperttunen, smangipudi, thierry.reding

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
v5 changes:
	* Updated ioremap_resource check as per Jon's comments.

 drivers/gpio/gpio-tegra186.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1bd9e44..8a64dcb 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
 
 	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
-	if (IS_ERR(gpio->secure))
-		return PTR_ERR(gpio->secure);
+	if (IS_ERR(gpio->secure)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(gpio->secure))
+			return PTR_ERR(gpio->secure);
+	}
 
 	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
-	if (IS_ERR(gpio->base))
-		return PTR_ERR(gpio->base);
+	if (IS_ERR(gpio->base)) {
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(gpio->base))
+			return PTR_ERR(gpio->base);
+	}
 
 	err = platform_irq_count(pdev);
 	if (err < 0)
@@ -690,11 +696,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
+#if defined(CONFIG_OF_GPIO)
 	gpio->gpio.of_node = pdev->dev.of_node;
 	gpio->gpio.of_gpio_n_cells = 2;
 	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+#endif /* CONFIG_OF_GPIO */
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	gpio->intc.name = dev_name(&pdev->dev);
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -918,10 +926,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = tegra186_gpio_acpi_match,
 	},
 	.probe = tegra186_gpio_probe,
 	.remove = tegra186_gpio_remove,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v5] gpio: tegra186: Add ACPI support
  2021-07-01  9:00                     ` [PATCH v5] " Akhil R
@ 2021-07-08 13:08                       ` Jon Hunter
  2021-07-16  8:31                       ` Bartosz Golaszewski
  1 sibling, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2021-07-08 13:08 UTC (permalink / raw)
  To: Akhil R
  Cc: andy.shevchenko, bgolaszewski, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding


On 01/07/2021 10:00, Akhil R wrote:
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> v5 changes:
> 	* Updated ioremap_resource check as per Jon's comments.
> 
>  drivers/gpio/gpio-tegra186.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index 1bd9e44..8a64dcb 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>  	if (!gpio)
>  		return -ENOMEM;
>  
> -	gpio->soc = of_device_get_match_data(&pdev->dev);
> +	gpio->soc = device_get_match_data(&pdev->dev);
>  
>  	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> -	if (IS_ERR(gpio->secure))
> -		return PTR_ERR(gpio->secure);
> +	if (IS_ERR(gpio->secure)) {
> +		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(gpio->secure))
> +			return PTR_ERR(gpio->secure);
> +	}
>  
>  	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
> -	if (IS_ERR(gpio->base))
> -		return PTR_ERR(gpio->base);
> +	if (IS_ERR(gpio->base)) {
> +		gpio->base = devm_platform_ioremap_resource(pdev, 1);
> +		if (IS_ERR(gpio->base))
> +			return PTR_ERR(gpio->base);
> +	}
>  
>  	err = platform_irq_count(pdev);
>  	if (err < 0)
> @@ -690,11 +696,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>  
>  	gpio->gpio.names = (const char * const *)names;
>  
> +#if defined(CONFIG_OF_GPIO)
>  	gpio->gpio.of_node = pdev->dev.of_node;
>  	gpio->gpio.of_gpio_n_cells = 2;
>  	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
> +#endif /* CONFIG_OF_GPIO */
>  
> -	gpio->intc.name = pdev->dev.of_node->name;
> +	gpio->intc.name = dev_name(&pdev->dev);
>  	gpio->intc.irq_ack = tegra186_irq_ack;
>  	gpio->intc.irq_mask = tegra186_irq_mask;
>  	gpio->intc.irq_unmask = tegra186_irq_unmask;
> @@ -918,10 +926,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
>  
> +static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
> +	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
> +	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
> +	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
> +	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
> +
>  static struct platform_driver tegra186_gpio_driver = {
>  	.driver = {
>  		.name = "tegra186-gpio",
>  		.of_match_table = tegra186_gpio_of_match,
> +		.acpi_match_table = tegra186_gpio_acpi_match,
>  	},
>  	.probe = tegra186_gpio_probe,
>  	.remove = tegra186_gpio_remove,
> 

Looks good to me.

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5] gpio: tegra186: Add ACPI support
  2021-07-01  9:00                     ` [PATCH v5] " Akhil R
  2021-07-08 13:08                       ` Jon Hunter
@ 2021-07-16  8:31                       ` Bartosz Golaszewski
  2021-07-19  4:46                         ` [PATCH v6] " Akhil R
  1 sibling, 1 reply; 19+ messages in thread
From: Bartosz Golaszewski @ 2021-07-16  8:31 UTC (permalink / raw)
  To: Akhil R
  Cc: Jonathan Hunter, Andy Shevchenko, kyarlagadda, Laxman Dewangan,
	Linus Walleij, linux-gpio, LKML, linux-tegra, mperttunen,
	smangipudi, Thierry Reding

On Thu, Jul 1, 2021 at 11:01 AM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> v5 changes:
>         * Updated ioremap_resource check as per Jon's comments.
>
>  drivers/gpio/gpio-tegra186.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index 1bd9e44..8a64dcb 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -620,15 +620,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>         if (!gpio)
>                 return -ENOMEM;
>
> -       gpio->soc = of_device_get_match_data(&pdev->dev);
> +       gpio->soc = device_get_match_data(&pdev->dev);
>
>         gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
> -       if (IS_ERR(gpio->secure))
> -               return PTR_ERR(gpio->secure);
> +       if (IS_ERR(gpio->secure)) {
> +               gpio->secure = devm_platform_ioremap_resource(pdev, 0);
> +               if (IS_ERR(gpio->secure))
> +                       return PTR_ERR(gpio->secure);
> +       }
>
>         gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
> -       if (IS_ERR(gpio->base))
> -               return PTR_ERR(gpio->base);
> +       if (IS_ERR(gpio->base)) {
> +               gpio->base = devm_platform_ioremap_resource(pdev, 1);
> +               if (IS_ERR(gpio->base))
> +                       return PTR_ERR(gpio->base);
> +       }
>
>         err = platform_irq_count(pdev);
>         if (err < 0)
> @@ -690,11 +696,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>
>         gpio->gpio.names = (const char * const *)names;
>
> +#if defined(CONFIG_OF_GPIO)
>         gpio->gpio.of_node = pdev->dev.of_node;
>         gpio->gpio.of_gpio_n_cells = 2;
>         gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
> +#endif /* CONFIG_OF_GPIO */
>
> -       gpio->intc.name = pdev->dev.of_node->name;
> +       gpio->intc.name = dev_name(&pdev->dev);
>         gpio->intc.irq_ack = tegra186_irq_ack;
>         gpio->intc.irq_mask = tegra186_irq_mask;
>         gpio->intc.irq_unmask = tegra186_irq_unmask;
> @@ -918,10 +926,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
>
> +static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
> +       { .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
> +       { .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
> +       { .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
> +       { .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
> +
>  static struct platform_driver tegra186_gpio_driver = {
>         .driver = {
>                 .name = "tegra186-gpio",
>                 .of_match_table = tegra186_gpio_of_match,
> +               .acpi_match_table = tegra186_gpio_acpi_match,
>         },
>         .probe = tegra186_gpio_probe,
>         .remove = tegra186_gpio_remove,
> --
> 2.7.4
>

Can you rebase it on top of v5.14-rc1 and resend?

Bart

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6] gpio: tegra186: Add ACPI support
  2021-07-16  8:31                       ` Bartosz Golaszewski
@ 2021-07-19  4:46                         ` Akhil R
  2021-08-05 19:30                           ` Bartosz Golaszewski
  0 siblings, 1 reply; 19+ messages in thread
From: Akhil R @ 2021-07-19  4:46 UTC (permalink / raw)
  To: bgolaszewski
  Cc: akhilrajeev, andy.shevchenko, jonathanh, kyarlagadda, ldewangan,
	linus.walleij, linux-gpio, linux-kernel, linux-tegra, mperttunen,
	smangipudi, thierry.reding

Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
---
v6 changes:
	* Rebased on top of v5.14-rc1

 drivers/gpio/gpio-tegra186.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index d38980b..046b7c8 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -610,15 +610,21 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	gpio->soc = of_device_get_match_data(&pdev->dev);
+	gpio->soc = device_get_match_data(&pdev->dev);
 
 	gpio->secure = devm_platform_ioremap_resource_byname(pdev, "security");
-	if (IS_ERR(gpio->secure))
-		return PTR_ERR(gpio->secure);
+	if (IS_ERR(gpio->secure)) {
+		gpio->secure = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(gpio->secure))
+			return PTR_ERR(gpio->secure);
+	}
 
 	gpio->base = devm_platform_ioremap_resource_byname(pdev, "gpio");
-	if (IS_ERR(gpio->base))
-		return PTR_ERR(gpio->base);
+	if (IS_ERR(gpio->base)) {
+		gpio->base = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(gpio->base))
+			return PTR_ERR(gpio->base);
+	}
 
 	err = platform_irq_count(pdev);
 	if (err < 0)
@@ -680,11 +686,13 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 
 	gpio->gpio.names = (const char * const *)names;
 
+#if defined(CONFIG_OF_GPIO)
 	gpio->gpio.of_node = pdev->dev.of_node;
 	gpio->gpio.of_gpio_n_cells = 2;
 	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+#endif /* CONFIG_OF_GPIO */
 
-	gpio->intc.name = pdev->dev.of_node->name;
+	gpio->intc.name = dev_name(&pdev->dev);
 	gpio->intc.irq_ack = tegra186_irq_ack;
 	gpio->intc.irq_mask = tegra186_irq_mask;
 	gpio->intc.irq_unmask = tegra186_irq_unmask;
@@ -896,10 +904,20 @@ static const struct of_device_id tegra186_gpio_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra186_gpio_of_match);
 
+static const struct acpi_device_id  tegra186_gpio_acpi_match[] = {
+	{ .id = "NVDA0108", .driver_data = (kernel_ulong_t)&tegra186_main_soc },
+	{ .id = "NVDA0208", .driver_data = (kernel_ulong_t)&tegra186_aon_soc },
+	{ .id = "NVDA0308", .driver_data = (kernel_ulong_t)&tegra194_main_soc },
+	{ .id = "NVDA0408", .driver_data = (kernel_ulong_t)&tegra194_aon_soc },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, tegra186_gpio_acpi_match);
+
 static struct platform_driver tegra186_gpio_driver = {
 	.driver = {
 		.name = "tegra186-gpio",
 		.of_match_table = tegra186_gpio_of_match,
+		.acpi_match_table = tegra186_gpio_acpi_match,
 	},
 	.probe = tegra186_gpio_probe,
 };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v6] gpio: tegra186: Add ACPI support
  2021-07-19  4:46                         ` [PATCH v6] " Akhil R
@ 2021-08-05 19:30                           ` Bartosz Golaszewski
  0 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2021-08-05 19:30 UTC (permalink / raw)
  To: Akhil R
  Cc: Andy Shevchenko, Jonathan Hunter, kyarlagadda, Laxman Dewangan,
	Linus Walleij, linux-gpio, LKML, linux-tegra, mperttunen,
	smangipudi, Thierry Reding

On Mon, Jul 19, 2021 at 6:47 AM Akhil R <akhilrajeev@nvidia.com> wrote:
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> v6 changes:
>         * Rebased on top of v5.14-rc1
>
>  drivers/gpio/gpio-tegra186.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
>

Applied, thanks!

Bartosz

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-08-05 19:31 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 10:05 [PATCH] gpio: tegra186: Add ACPI support Akhil R
2021-06-17 14:54 ` Linus Walleij
2021-06-17 15:21 ` Andy Shevchenko
2021-06-18 13:39   ` [PATCH v2] " Akhil R
2021-06-18 14:31     ` Andy Shevchenko
2021-06-21 15:08       ` Akhil R
2021-06-29 15:17         ` Akhil R
2021-06-30 16:14         ` Andy Shevchenko
2021-06-30 17:47           ` Akhil R
2021-06-30 18:17             ` [PATCH v3] " Akhil R
2021-06-30 18:23               ` Andy Shevchenko
2021-07-01  5:01                 ` [PATCH v4] " Akhil R
2021-07-01  5:53                   ` Jon Hunter
2021-07-01  9:00                     ` [PATCH v5] " Akhil R
2021-07-08 13:08                       ` Jon Hunter
2021-07-16  8:31                       ` Bartosz Golaszewski
2021-07-19  4:46                         ` [PATCH v6] " Akhil R
2021-08-05 19:30                           ` Bartosz Golaszewski
2021-06-18 19:39 ` [PATCH] " kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).