2023-10-11 11:22:29

by Kartik Rajput

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

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