Re: [PATCH v10 3/3] HID: cp2112: Fwnode Support

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Feb 05, 2024 at 11:09:22AM -0600, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
> 
> I2C and GPIO child nodes can either be children with names "i2c" and
> "gpio", or, for ACPI, device nodes with _ADR Zero and One,
> respectively.

> Additionally, support configuring the I2C bus speed from the
> clock-frequency device property.

This has to be in a separate patch, which may predecess this one.

...

> +		name = fwnode_get_name(child);
> +		if (name) {
> +			ret = match_string(cp2112_cell_names,
> +							ARRAY_SIZE(cp2112_cell_names), name);
> +			if (ret >= 0)
> +				addr = ret;
> +		}
> +		if (!name || ret < 0)
> +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +
> +		if (ret < 0)
> +			continue;

I don't like this piece (esp. due to possible matching with node name which
may not be so reliable), but I have no better solution right now.

Maybe this way (this doesn't particularly solve the issue but seems better
to me)?

		ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
		if (ret) {
			/* If no ACPI given or compiled, fallback to matching names */
			name = fwnode_get_name(child);
			if (!name)
				continue;

			ret = match_string(cp2112_cell_names, ARRAY_SIZE(cp2112_cell_names), name);
			if (ret < 0)
				continue;

			addr = ret;
		}


-- 
With Best Regards,
Andy Shevchenko