Re: [PATCH v4 5/8] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS

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

On Wed, Feb 08, 2023 at 05:28:04PM +0200, Sakari Ailus wrote:
> Dig "rotation" property value for devices with _CRS CSI2 resource
> descriptor. The value comes from _PLD (physical location of device)
> object, if it exists for the device.
> 
> This way camera sensor drivers that know the "rotation" property do not
> need to care about _PLD on ACPI.

...

> +	/*
> +	 * Check if "rotation" property exists and if it doesn't but there's a
> +	 * _PLD object, then get the rotation value from there.
> +	 */
> +	if (fwnode_property_read_u32(fwnode, "rotation", &val) &&
> +	    ACPI_SUCCESS(acpi_get_physical_device_location(acpi_device_handle(device),
> +							   &pld))) {

> +	if (fwnode_property_read_u32(fwnode, "rotation", &val) &&
> +	    ACPI_SUCCESS(acpi_get_physical_device_location(acpi_device_handle(device),
> +							   &pld))) {

Wouldn't be a bit better to use temporary variables for this?

	ret = fwnode_property_read_u32(fwnode, "rotation", &val);
	if (ret) {
		acpi_handle handle = acpi_device_handle(device);
		acpi_status status;

		status = acpi_get_physical_device_location(handle, &pld);
		if (ACPI_SUCCESS(status)) {
			...
		}
	}

?

> +		ads->dev_props[NEXT_PROPERTY(prop_index, DEV_ROTATION)] =
> +			PROPERTY_ENTRY_U32("rotation", pld->rotation * 45U);
> +		kfree(pld);
> +	}


-- 
With Best Regards,
Andy Shevchenko