Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45172 )
Change subject: drivers/intel/mipi_camera: Add support for dynamic SSDT generation ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45172/8/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera_sku.c:
https://review.coreboot.org/c/coreboot/+/45172/8/src/drivers/intel/mipi_came... PS8, Line 36: /* By default device will be set to enabled in alloc_dev. So calling dev_set_enabled : is not making call to chip_ops->enable_dev as device is already enabled */ : dev_set_enabled(sensor, 0); : dev_set_enabled(sensor, 1); : }
This is a little confusing. […]
On calling alloc_dev, it sets the enabled flag in device to 1. https://github.com/coreboot/coreboot/blob/b8b117c7e72ceb641c14db499a2c004fdf...
Due to this, on calling dev_set_enabled once, it returns without doing anything, as the device is already set to enabled. https://github.com/coreboot/coreboot/blob/b8b117c7e72ceb641c14db499a2c004fdf...
As following functions in dev_set_enabled are not called, camera_enable in camera.c is never called. dev->ops->enable(dev) dev->chip_ops && dev->chip_ops->enable_dev(dev)
https://github.com/coreboot/coreboot/blob/b8b117c7e72ceb641c14db499a2c004fdf...
So camera_ssdt_fill(which is responsible for the SSDT generation for the device) will not be called from acpi.c as dev->ops is not initialized. https://github.com/coreboot/coreboot/blob/b8b117c7e72ceb641c14db499a2c004fdf...
So it is required to disable once and then enable. I will update the comment.