Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.112.34 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.112.34; helo=mail.nvidia.com;
From:   Akhil R <akhilrajeev@nvidia.com>
To:     <andy.shevchenko@gmail.com>
CC:     <akhilrajeev@nvidia.com>, <bgolaszewski@baylibre.com>,
        <jonathanh@nvidia.com>, <kyarlagadda@nvidia.com>,
        <ldewangan@nvidia.com>, <linus.walleij@linaro.org>,
        <linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-tegra@vger.kernel.org>, <mperttunen@nvidia.com>,
        <smangipudi@nvidia.com>, <thierry.reding@gmail.com>
Subject: Re: [PATCH v2] gpio: tegra186: Add ACPI support
Date:   Mon, 21 Jun 2021 20:38:09 +0530
Message-ID: <1624288089-15278-1-git-send-email-akhilrajeev@nvidia.com>
In-Reply-To: <CAHp75Vf+8t78NbYeCh+VLEAzcYJxDP9Pz7DPW_MC9D-K_Ff8uA@mail.gmail.com>
References: <CAHp75Vf+8t78NbYeCh+VLEAzcYJxDP9Pz7DPW_MC9D-K_Ff8uA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jun 2021 15:07:50.8674
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 38cf5f20-90b1-4d18-1fa9-08d934c6568d
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.112.34];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT026.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB2859
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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