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

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: <ZP7wDm5w/b5RIlIq@smile.fi.intel.com>
Date:   Mon, 11 Sep 2023 13:46:38 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Kartik <kkartik@...dia.com>
Cc:     thierry.reding@...il.com, jonathanh@...dia.com, sumitg@...dia.com,
        arnd@...db.de, pshete@...dia.com, digetx@...il.com,
        petlozup@...dia.com, windhl@....com, frank.li@...o.com,
        robh@...nel.org, stefank@...dia.com, pdeschrijver@...dia.com,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/6] soc/tegra: fuse: Add ACPI support for Tegra194
 and Tegra234

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?

...

> +	if (is_acpi_node(pdev->dev.fwnode) && !fuse->soc) {

We do not dereference fwnode in struct device, use dev_fwnode() for that.

...

> +		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.

...

> +	/*
> +	 * 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?

> +		return -EPROBE_DEFER;

-- 
With Best Regards,
Andy Shevchenko