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

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

On Mon, Jan 30, 2023 at 11:22:00PM +0200, Sakari Ailus wrote:
> On Fri, Jan 27, 2023 at 12:35:12PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 26, 2023 at 12:40:57AM +0200, Sakari Ailus wrote:

...

> > > +static struct fwnode_handle *get_mipi_port_handle(struct acpi_device *device,
> > > +						  unsigned int port)
> > > +{
> > > +	static const char mipi_port_prefix[] = "mipi-img-port-";
> > > +	char mipi_port_name[sizeof(mipi_port_prefix) + 2];
> > 
> > I think the following will be better:
> > 
> > 	char mipi_port_name[16];
> 
> If the array is too short, this will generate a warning but... this is
> already handled better than that. Why not to keep it?

We will get the format string self contained.

> > > +	if (snprintf(mipi_port_name, sizeof(mipi_port_name), "%s%u",
> > > +		     mipi_port_prefix, port) >= sizeof(mipi_port_name)) {
> > 
> > 	if (snprintf(mipi_port_name, sizeof(mipi_port_name), "mipi-img-port-%u",
> > 		     port) >= sizeof(mipi_port_name)) {
> > 
> > > +		acpi_handle_info(acpi_device_handle(device),
> > > +				 "mipi port name too long for port %u\n", port);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	return fwnode_get_named_child_node(acpi_fwnode_handle(device),
> > > +					   mipi_port_name);
> > > +}

...

> > > +	union {
> > > +		u32 val;
> > 
> > OK. I stopped here, because I'm not sure you have really sent an updated
> > version. For sure the val and union is not used.
> 
> Indeed. My apologies --- I missed making a few changes I intended to do.
> I'll go through those once more and then send v4.

Thank you!

> > > +		/* Data lanes + the clock lane */
> > > +		u8 val8[BITS_TO_BYTES(ARRAY_SIZE(port->data_lanes) + 1)];
> > > +	} u;

-- 
With Best Regards,
Andy Shevchenko