Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41607 )
Change subject: drivers/intel/mipi_camera: camera SSDT generation ......................................................................
Patch Set 11:
(6 comments)
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 40: = 0;
no need to initialize.
will update this in next patch.
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 42: struct acpi_dp *ep_table = NULL; : struct acpi_dp *port_table = NULL; : struct acpi_dp *remote = NULL;
Initializing these as NULL only makes sense if you do a NULL check after acpi_dp_new_table() which d […]
acpi_dp_new_table is internally calling malloc which calls die incase if it unable to allocated any memory. Memory allocation is unlikely to fail in coreboot. Initial version of this patch had these checks and error handling. But after discussing with @Matt it was removed.
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 48: strdup
check for NULL?
As mentioned above strdup is calling malloc
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 50: acpi_dp_add_integer
It seems uncommon to do but do you want to check the return value?
acpi_dp_add_integer is calling acpi_dp_new which calls malloc
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 91: *config
check for NULL.
camera_fill_ssdt is be called only if required configs are enabled in devicetree. So dev->chip_info will always be initialized? May be we can check in camera_fill_ssdt so that we will not enter any of these function if chip_info is not set.
https://review.coreboot.org/c/coreboot/+/41607/11/src/drivers/intel/mipi_cam... PS11, Line 216: config
check for NULL.
As mentioned above.