Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.118.232 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.118.232; helo=mail.nvidia.com; pr=C
From:   Kartik <kkartik@nvidia.com>
To:     <andriy.shevchenko@linux.intel.com>
CC:     <arnd@arndb.de>, <digetx@gmail.com>, <frank.li@vivo.com>,
        <jonathanh@nvidia.com>, <kkartik@nvidia.com>,
        <linux-kernel@vger.kernel.org>, <linux-tegra@vger.kernel.org>,
        <pdeschrijver@nvidia.com>, <petlozup@nvidia.com>,
        <pshete@nvidia.com>, <robh@kernel.org>, <stefank@nvidia.com>,
        <sumitg@nvidia.com>, <thierry.reding@gmail.com>, <windhl@126.com>
Subject: Re: [PATCH v3 5/6] soc/tegra: fuse: Add ACPI support for Tegra194 and Tegra234
Date:   Wed, 11 Oct 2023 15:08:32 +0530
Message-ID: <20231011093832.8095-1-kkartik@nvidia.com>
In-Reply-To: <ZP7wDm5w/b5RIlIq@smile.fi.intel.com>
References: <ZP7wDm5w/b5RIlIq@smile.fi.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2023 11:20:55.1278
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 997de3bf-b9a7-4edf-a436-08dbca4c22cb
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.118.232];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: MN1PEPF0000F0E3.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR12MB9435
X-Spam-Status: Yes, score=5.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,SORTED_RECIPS,SPF_HELO_NONE,
	SPF_PASS autolearn=no autolearn_force=no version=3.4.6
X-Spam-Level: *****
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Wed, 11 Oct 2023 04:22:27 -0700 (PDT)
X-Spam-Report: 
	*  3.6 RCVD_IN_SBL_CSS RBL: Received via a relay in Spamhaus SBL-CSS
	*      [2620:137:e000:0:0:0:3:0 listed in]
	[zen.spamhaus.org]
	*  0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record
	*  0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level
	*      mail domains are different
	*  2.5 SORTED_RECIPS Recipient list is sorted by address
	* -0.0 SPF_PASS SPF: sender matches SPF record
	* -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature
	* -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from
	*      author's domain
	*  0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily
	*       valid
	* -1.0 MAILING_LIST_MULTI Multiple indicators imply a widely-seen list
	*       manager
	* -0.0 DKIMWL_WL_HIGH DKIMwl.org - High trust sender

On Mon, 2023-09-11 at 10:46, Andy Shevchenko wrote:
>On Thu, Sep 07, 2023 at 12:40:51PM +0530, Kartik wrote:
>> Add ACPI support for Tegra194 & Tegra243 SoC's. This requires
>> following modifications to the probe when ACPI boot is used:
>>  - Initialize soc data.
>>  - Add nvmem lookups.
>>  - Register soc device.
>>  - use devm_clk_get_optional() instead of devm_clk_get() to get
>>    fuse->clk, as fuse clocks are not required when using ACPI boot.
>> 
>> Also, drop '__init' keyword for tegra_soc_device_register() as this is also
>> used by tegra_fuse_probe() and use dev_err_probe() wherever applicable.
>
>...
>
>> +static const struct acpi_device_id tegra_fuse_acpi_match[] = {
>> +	{ "NVDA200F" },
>> +	{ /* sentinel */ }
>> +};
>
>Where is NODULE_DEVICE_TABLE() for it? And why it's located here and not
>at the its first user's location?

Fixed this in v4.

>
>...
>
>> +	if (is_acpi_node(pdev->dev.fwnode) && !fuse->soc) {
>
>We do not dereference fwnode in struct device, use dev_fwnode() for that.
>
>...
>

Fixed this in v4.

>> +		return dev_err_probe(&pdev->dev, PTR_ERR(fuse->clk), "failed to get FUSE clock\n");
>
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(fuse->rst), "failed to get FUSE reset");
>
>This requires a separate patch.
>

Pushed a separate patch in v4 of this series.

>...
>
>> +	/*
>> +	 * Wait for fuse->clk to be initialized if device-tree boot is used.
>> +	 */
>
>> +	if (is_of_node(fuse->dev->fwnode) && !fuse->clk)
>
>dev_fwnode()
>
>The second check is questionable. If you get it NULL, it won't ever appear.
>Did I miss anything?
>

If tegra_fuse_readl is called before Tegra fuse driver is probed, then
fuse->clk will NULL. Hence, this is required here.

>> +		return -EPROBE_DEFER;
>
>-- 
>With Best Regards,
>Andy Shevchenko

Regards,
Kartik