On Thu, Jan 26, 2023 at 12:40:57AM +0200, Sakari Ailus wrote:
> Generate software nodes for information parsed from ACPI _CRS for CSI-2 as
> well as MIPI DisCo for Imaging spec. The software nodes are compliant with
> existing ACPI or DT definitions and are parsed by relevant drivers without
> changes.
...
> +static unsigned int next_csi2_port_index(struct acpi_device_software_nodes *ads,
> + unsigned int port_nr)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ads->num_ports; i++) {
> + struct acpi_device_software_node_port *port = &ads->ports[i];
> +
> + if (port->port_nr == port_nr)
> + return i;
> + if (port->port_nr != NO_CSI2_PORT)
> + continue;
> +
> + port->port_nr = port_nr;
> +
> + return i;
Maybe it would be better to use the same pattern as above?
if (port->port_nr == NO_CSI2_PORT) {
port->port_nr = port_nr;
return i;
}
> + }
> +
> + return NO_CSI2_PORT;
> +}
...
> +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 (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.
> + /* Data lanes + the clock lane */
> + u8 val8[BITS_TO_BYTES(ARRAY_SIZE(port->data_lanes) + 1)];
> + } u;
...
Please, address all in v3.
--
With Best Regards,
Andy Shevchenko
