Messages in this thread | | | From | Andy Shevchenko <> | Date | Fri, 18 Jun 2021 17:31:23 +0300 | Subject | Re: [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
| |