linux-kernel - Re: [PATCH v12 2/3] HID: cp2112: Fwnode Support

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: <aSdGh3i_KYocE3L3@smile.fi.intel.com>
Date: Wed, 26 Nov 2025 20:27:19 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Danny Kaehn <danny.kaehn@...xus.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Benjamin Tissoires <bentiss@...nel.org>,
	Andi Shyti <andi.shyti@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Jiri Kosina <jikos@...nel.org>,
	devicetree@...r.kernel.org, linux-input@...r.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
	Ethan Twardy <ethan.twardy@...xus.com>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org, Leo Huang <leohu@...dia.com>,
	Arun D Patil <arundp@...dia.com>, Willie Thai <wthai@...dia.com>,
	Ting-Kai Chen <tingkaic@...dia.com>
Subject: Re: [PATCH v12 2/3] HID: cp2112: Fwnode Support

On Wed, Nov 26, 2025 at 11:05:25AM -0600, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
> 
> Bindings between the firmware nodes and the functions of the device
> are distinct between ACPI and DeviceTree.
> 
> For ACPI, the i2c_adapter will use the child with _ADR Zero and the
> gpio_chip will use the child with _ADR One. For DeviceTree, the
> i2c_adapter will use the child with name "i2c", but the gpio_chip
> will share a firmware node with the CP2112.

Hmm... Is there any explanation why DT decided to go that way?

...

> +	if (is_acpi_device_node(hdev->dev.fwnode)) {

Please, do not dereference fwnode, use dev_fwnode() or other APIs for that
(actually the same applies to OF node, but people too much neglect that).

> +		device_for_each_child_node(&hdev->dev, child) {
> +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +			if (ret)
> +				continue;
> +
> +			switch (addr) {
> +			case CP2112_I2C_ADR:
> +				device_set_node(&dev->adap.dev, child);
> +				break;
> +			case CP2112_GPIO_ADR:
> +				dev->gc.fwnode = child;
> +				break;

If by any chance we have malformed table and there are more devices with
the same address? Maybe we don't need to address this right now, just
asking... (I believe ACPI compiler won't allow that, but table can be
crafted directly in the binary format.)

> +			}
> +		}
> +	} else {
> +		device_set_node(&dev->adap.dev,
> +			device_get_named_child_node(&hdev->dev, "i2c"));

Here we bump the reference count, where is it going to be dropped?

Note, in the other branch (ACPI) the reference count is not bumped in
the current code.

> +	}

-- 
With Best Regards,
Andy Shevchenko