Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41607 )
Change subject: drivers/intel/mipi_camera: camera SSDT generation ......................................................................
Patch Set 12:
(13 comments)
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG@7 PS12, Line 7: drivers/intel/mipi_camera: camera SSDT generation Please make it a statement by adding a verb (in imperative mood).
Generate SSDT for camera
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG@12 PS12, Line 12: missing some properties needed for Linux drivers. Please mention the properties.
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG@9 PS12, Line 9: Some boards & variants are using raw asl files to define the ACPI devices : (and related properties) required by Linux MIPI camera drivers. : The mipi_camera driver can provide a SSDB property, but it's : missing some properties needed for Linux drivers. Please re-flow for 75 characters per line.
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG@21 PS12, Line 21: asl ASL
https://review.coreboot.org/c/coreboot/+/41607/12//COMMIT_MSG@22 PS12, Line 22: Please mention one difference.
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 42: uint32_t i, j; Please use the native types `unsigned int` or `size_t`.
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 607: prefix = "ERR"; /* Error */ Should a debug message be added for this case?
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 612: know knows?
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 131: * General settings that can be used for any type (but doesn't don’t?
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 132: * necessarily need to be used/specified for each device. Missing closing ).
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 148: uint8_t num_freq_entries; /* # of elements in link_freq */ Please use native types.
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 163: * you can use zero for a value. Please re-flow for at least 80 characters per line (maybe even 96).
https://review.coreboot.org/c/coreboot/+/41607/12/src/drivers/intel/mipi_cam... PS12, Line 167: uint32_t nvm_size; Can native types be used here?