Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41624 )
Change subject: drivers/intel/mipi_camera: Support for adding camera power resource ......................................................................
Patch Set 12:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... PS12, Line 428: int unsigned int
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... PS12, Line 445: printk(BIOS_ERR, "Unsupported clock action: %x\n", Please add the consequence for the user. Something like: *aborting camera setup* or *OS camera driver will likely not work*.
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... PS12, Line 458: printk(BIOS_ERR, "Unsupported GPIO action: %x\n", Please add the consequence for the user. Something like: *aborting camera setup* or *OS camera driver will likely not work*.
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... PS12, Line 464: printk(BIOS_ERR, "Unsupported power operation: %x\n", seq->ops[i].type); Please add the consequence for the user. Something like: *aborting camera setup* or *OS camera driver will likely not work*.
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/41624/12/src/drivers/intel/mipi_cam... PS12, Line 91: enum action_type action; Action type is not a good name? Why can’t a bool be used, and named `enable`?