| Message ID | 1623924351-22489-1-git-send-email-akhilrajeev@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | gpio: tegra186: Add ACPI support | expand |
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
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.
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
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,