lkml.org 
[lkml]   [2021]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2] gpio: tegra186: Add ACPI support
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

\
 
 \ /
  Last update: 2021-06-18 16:32    [W:2.338 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site