[libcamera-devel] Fwd: Surface Go VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go (version1))

[Andy Shevchenko]

On Mon, Nov 08, 2021 at 02:12:38PM +0100, Hans de Goede wrote:
> On 11/2/21 00:43, Daniel Scally wrote:

> Does this sound reasonable / like I'm heading in the right direction?

It is up to you folks, since I have no time to participate in this with
a full dive right now. Below just some comments on the patches in case
they will go.

...

> -	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	struct acpi_device *adev = to_acpi_device_node(fwnode);
>  	struct i2c_acpi_lookup lookup;
>  	struct i2c_adapter *adapter;
>  	LIST_HEAD(resource_list);
>  	int ret;

Make sense to move assignment here.

	adev = to_acpi_device_node(fwnode);

> +	if (!adev)
> +		return ERR_PTR(-ENODEV);

...

> +static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
> +						     int index,
> +						     struct i2c_board_info *info)
> +{
> +	return i2c_acpi_new_device_by_fwnode(dev->fwnode, index, info);

dev_fwnode(dev)

> +}

...

> +int cio2_bridge_sensors_are_ready(void)
> +{
> +	struct acpi_device *adev;

> +	bool ready = true;

Redundant. See below.

> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(cio2_supported_sensors); i++) {
> +		const struct cio2_sensor_config *cfg =
> +			&cio2_supported_sensors[i];
> +
> +		for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
> +			if (!adev->status.enabled)
> +				continue;

> +			if (!acpi_dev_ready_for_enumeration(adev))
> +				ready = false;

You may put the adev here and return false.

> +		}
> +	}

> +	return ready;

So return true.

> +}

...

> +	if (sensor->ssdb.vcmtype)
> +		nodes[SWNODE_VCM] = NODE_VCM(
> +					cio2_vcm_types[sensor->ssdb.vcmtype - 1]);

Wouldn't be better

		nodes[SWNODE_VCM] =
			NODE_VCM(cio2_vcm_types[sensor->ssdb.vcmtype - 1]);

?

...

> +	sensor->vcm_i2c_client = i2c_acpi_new_device_by_fwnode(
> +					acpi_fwnode_handle(sensor->adev),
> +					1, &board_info);

Ditto.

-- 
With Best Regards,
Andy Shevchenko