linux-kernel - Re: [PATCH v2] gpio: tegra186: Add ACPI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1624288089-15278-1-git-send-email-akhilrajeev@nvidia.com>
Date:   Mon, 21 Jun 2021 20:38:09 +0530
From:   Akhil R <akhilrajeev@...dia.com>
To:     <andy.shevchenko@...il.com>
CC:     <akhilrajeev@...dia.com>, <bgolaszewski@...libre.com>,
        <jonathanh@...dia.com>, <kyarlagadda@...dia.com>,
        <ldewangan@...dia.com>, <linus.walleij@...aro.org>,
        <linux-gpio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-tegra@...r.kernel.org>, <mperttunen@...dia.com>,
        <smangipudi@...dia.com>, <thierry.reding@...il.com>
Subject: Re: [PATCH v2] gpio: tegra186: Add ACPI support


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