From: Akhil Rajeev <[email protected]>
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Signed-off-by: Akhil Rajeev <[email protected]>
---
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 <[email protected]>
*/
+#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
Hi Akhil,
thanks for your patch!
On Thu, Jun 17, 2021 at 12:05 PM Akhil R <[email protected]> wrote:
> From: Akhil Rajeev <[email protected]>
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil Rajeev <[email protected]>
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
On Thu, Jun 17, 2021 at 1:18 PM Akhil R <[email protected]> wrote:
>
> From: Akhil Rajeev <[email protected]>
>
> 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
From: Akhil Rajeev <[email protected]>
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Signed-off-by: Akhil Rajeev <[email protected]>
---
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 <[email protected]>
*/
+#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
On Fri, Jun 18, 2021 at 4:39 PM Akhil R <[email protected]> wrote:
Thanks for update, my comments below.
> From: Akhil Rajeev <[email protected]>
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
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 <[email protected]>
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/[email protected]
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
Hi Andy,
Please let me know if you have any inputs regarding the changes.
--
Best Regards,
Akhil
On Mon, Jun 21, 2021 at 6:07 PM Akhil R <[email protected]> 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
> > >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
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Change-Id: Id8e892f989e4ccc935b87aa0d84b10a3d1efd8f9
Signed-off-by: Akhil Rajeev <[email protected]>
---
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
On Wed, Jun 30, 2021 at 9:17 PM Akhil R <[email protected]> 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 <[email protected]>
> Change-Id: Id8e892f989e4ccc935b87aa0d84b10a3d1efd8f9
This is not for upstream.
> Signed-off-by: Akhil Rajeev <[email protected]>
...
> +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
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Signed-off-by: Akhil R <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
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
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 <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> 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
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Signed-off-by: Akhil R <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
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
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 <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> 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 <[email protected]>
Cheers
Jon
--
nvpublic
On Thu, Jul 1, 2021 at 11:01 AM Akhil R <[email protected]> wrote:
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil R <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> 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
Add ACPI module ID to probe the driver from the ACPI based bootloader
firmware.
Signed-off-by: Akhil R <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Jon Hunter <[email protected]>
---
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
On Mon, Jul 19, 2021 at 6:47 AM Akhil R <[email protected]> wrote:
>
> Add ACPI module ID to probe the driver from the ACPI based bootloader
> firmware.
>
> Signed-off-by: Akhil R <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Jon Hunter <[email protected]>
> ---
> 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