Re: [PATCH v2 4/8] ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging

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

On Wed, Jan 25, 2023 at 10:56:41AM +0200, Sakari Ailus wrote:
> On Tue, Jan 24, 2023 at 09:26:31PM +0200, Andy Shevchenko wrote:
> > On Mon, Jan 23, 2023 at 03:46:13PM +0200, Sakari Ailus wrote:

...

> > > +		if (num_lanes > ARRAY_SIZE(port->data_lanes)) {
> > 
> > 		>= MAX_LANES(port)
> 
> I find the original better: it does this by referring to the array itself.

Whatever, it's just an example to show where it's being used.

> > > +			acpi_handle_warn(acpi_device_handle(device),
> > > +					 "too many data lanes (%u)\n",
> > > +					 num_lanes);
> > > +			num_lanes = ARRAY_SIZE(port->data_lanes);
> > 
> > 			= MAX_LANES(port) - 1;
> > 
> > > +		}

...

> > 	ret = fwnode_property_count_u8(mipi_port_fwnode, "mipi-img-dlane-polarities");
> > 	if (ret < 0) {
> > 		acpi_handle_debug(acpi_device_handle(device),
> > 				  "no lane polarity provided\n");
> > 	} else if (ret < 1 + num_lanes) {
> > 		acpi_handle_warn(acpi_device_handle(device),
> > 				 "too few lane polarity bytes (%u)\n", bytes);
> > 	} else {
> > 		// assuming we dropped the union and renamed to val...
> > 		ret = fwnode_property_read_u8_array(mipi_port_fwnode,
> > 						    "mipi-img-lane-polarities",
> > 						    val, sizeof(val));
> > 		if (ret) {
> > 			...can't read... (debug message?)
> > 		} else {
> > 			unsigned int i;
> > 
> > 			for (i = 0; i < 1 + num_lanes; i++)
> > 				bitmap_set_value8(polarity, val[i], i * BITS_PER_BYTE);
> 
> You'll still needed to access invididual bits in val.

I didn't get this. The below is what it does in most efficient way.

> > 			// assuming that lane_polarities is zeroed by default...
> > 			for_each_set_bit(i, polarity, 1 + num_lanes)
> > 				port->lane_polarities[i] = 1;

Note that his code lacks of endianess issues.

> > 		}
> > 	}

-- 
With Best Regards,
Andy Shevchenko