Re: [PATCH v3 5/6] soc/tegra: fuse: Add ACPI support for Tegra194 and Tegra234

[Kartik <kkartik@nvidia.com>]

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