Change in coreboot[master]: drivers/intel/mipi_camera: camera SSDT generation

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41607 )

Change subject: drivers/intel/mipi_camera: camera SSDT generation
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... 
File src/drivers/intel/mipi_camera/camera.c:

https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... 
PS6, Line 188: 	/* Method (_DSM, 4, NotSerialized) */
             : 	acpigen_write_method("_DSM", 0x4);
             : 
             : 	/* ToBuffer (Arg0, Local0) */
             : 	acpigen_write_to_buffer(ARG0_OP, LOCAL0_OP);
             : 
             : 	/* If (LEqual (Local0, ToUUID(uuid))) */
             : 	acpigen_write_if();
             : 	acpigen_emit_byte(LEQUAL_OP);
             : 	acpigen_emit_byte(LOCAL0_OP);
             : 	acpigen_write_uuid("822ace8f-2814-4174-a56b-5f029fe079ee");
             : 	acpigen_write_return_string(config->sensor_name ? config->sensor_name : "UNKNOWN");
             : 	acpigen_pop_len();	/* If */
             : 
             : 	/* If (LEqual (Local0, ToUUID(uuid))) */
             : 	acpigen_write_if();
             : 	acpigen_emit_byte(LEQUAL_OP);
             : 	acpigen_emit_byte(LOCAL0_OP);
             : 	acpigen_write_uuid("26257549-9271-4ca4-bb43-c4899d5a4881");
             : 	/* ToInteger (Arg2, Local1) */
             : 	acpigen_write_to_integer(ARG2_OP, LOCAL1_OP);
             : 
             : 	/* If (LEqual (Local1, 1)) */
             : 	acpigen_write_if_lequal_op_int(LOCAL1_OP, 1);
             : 	acpigen_write_return_integer((config->ssdb.vcm_type ? 1 : 0) +
             : 				     (config->ssdb.rom_type ? 1 : 0) + 1);
             : 	acpigen_pop_len();	/* If Arg2=1 */
             : 
             : 	/* If (LEqual (Local1, 2)) */
             : 	acpigen_write_if_lequal_op_int(LOCAL1_OP, 2);
             : 	acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 |
             : 				     ((uint32_t)i2c_addr) << 8 | 0x0); /* type 0 for sensor */
             : 	acpigen_pop_len();	/* If Arg2=2 */
             : 
             : 	if (config->ssdb.vcm_type) {
             : 		/* If (LEqual (Local1, 3)) */
             : 		acpigen_write_if_lequal_op_int(LOCAL1_OP, 3);
             : 		acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 1 for vcm */
             : 					     ((uint32_t)config->vcm_address) << 8 | 0x01);
             : 		acpigen_pop_len();      /* If Arg2=3 */
             : 	}
             : 	if (config->ssdb.rom_type) {
             : 		/* If (LEqual (Local1, 3 or 4)) */
             : 		acpigen_write_if_lequal_op_int(LOCAL1_OP, 3 + (config->ssdb.vcm_type ? 1 : 0));
             : 		acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 2 for rom */
             : 					     ((uint32_t)config->rom_address) << 8 | 0x02);
             : 		acpigen_pop_len();      /* If Arg2=3 or 4 */
             : 	}
             : 
             : 	acpigen_pop_len();      /* If uuid */
             : 
             : 	/* Return (Buffer (One) { 0x0 }) */
             : 	acpigen_write_return_singleton_buffer(0x0);
             : 
             : 	acpigen_pop_len();      /* Method _DSM */
> acpigen_write_dsm_uuid_arr() could be used for "26257549-9271-4ca4-bb43-c4899d5a4881", though "822ac […]

They should be checking Arg2 as well; _DSM is a weird Method: "[Arg2] represents a specific function whose meaning is specific to the UUID and Revision ID." So you should be checking which Function is requested before returning anything.