Hello Matt Delco,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42466
to review the following change.
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
drivers/intel/mipi_camera: Handle NVM and VCM device type
This change adds support in mipi_camera driver to handle NVM and VCM device types.
Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 65 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/42466/1
diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 2f4b953..955c430 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,6 +10,46 @@ #include <device/pci_def.h> #include "chip.h"
+static void camera_fill_nvm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd = acpi_dp_new_table("_DSD"); + + /* It might be possible to default size or width based on type. */ + if (!config->disable_nvm_defaults && !config->nvm_pagesize) + config->nvm_pagesize = 1; + + if (!config->disable_nvm_defaults && !config->nvm_readonly) + config->nvm_readonly = 1; + + if (config->nvm_size) + acpi_dp_add_integer(dsd, "size", config->nvm_size); + + if (config->nvm_pagesize) + acpi_dp_add_integer(dsd, "pagesize", config->nvm_pagesize); + + if (config->nvm_readonly) + acpi_dp_add_integer(dsd, "read-only", config->nvm_readonly); + + if (config->nvm_width) + acpi_dp_add_integer(dsd, "address-width", config->nvm_width); + + acpi_dp_write(dsd); +} + +static void camera_fill_vcm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd; + + if (!config->vcm_compat) + return; + + dsd = acpi_dp_new_table("_DSD"); + acpi_dp_add_string(dsd, "compatible", config->vcm_compat); + acpi_dp_write(dsd); +} + static void write_pci_camera_device(const struct device *dev, const char *scope) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -100,6 +140,19 @@ config->device_type == INTEL_ACPI_CAMERA_VCM) { acpigen_write_name_integer("CAMD", config->device_type); } + + switch (config->device_type) { + case INTEL_ACPI_CAMERA_VCM: + camera_fill_vcm(dev); + break; + case INTEL_ACPI_CAMERA_NVM: + camera_fill_nvm(dev); + break; + case INTEL_ACPI_CAMERA_IMGU: + case INTEL_ACPI_CAMERA_PMIC: + default: + break; + } }
static void camera_fill_ssdt(const struct device *dev) diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index c029570..5aa2183 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -95,6 +95,18 @@ const char *vcm_name; /* defaults to |vcm_address| device */ uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ uint16_t vcm_address; /* I2C to use if ssdb.vcm_type != 0 */ + /* Settings specific to nvram. Many values, if left as zero, will be assigned a default. + Set disable_nvm_defaults to non-zero if you want to disable the defaulting behavior + so you can use zero for a value. */ + + bool disable_nvm_defaults; + uint32_t nvm_size; + uint32_t nvm_pagesize; + uint32_t nvm_readonly; + uint32_t nvm_width; + + /* Settings specific to vcm */ + const char *vcm_compat; };
#endif
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... PS1, Line 98: /* Settings specific to nvram. Many values, if left as zero, will be assigned a default. : Set disable_nvm_defaults to non-zero if you want to disable the defaulting behavior : so you can use zero for a value. */ /* * This is the preferred formatting for multiline comments * in coreboot */
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... PS1, Line 101: nit: skip the blank line after the comment, and add one before it starts
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42466
to look at the new patch set (#2).
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
drivers/intel/mipi_camera: Handle NVM and VCM device type
This change adds support in mipi_camera driver to handle NVM and VCM device types.
Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 66 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/42466/2
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42466
to look at the new patch set (#3).
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
drivers/intel/mipi_camera: Handle NVM and VCM device type
This change adds support in mipi_camera driver to handle NVM and VCM device types.
Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 66 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/42466/3
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... PS1, Line 98: /* Settings specific to nvram. Many values, if left as zero, will be assigned a default. : Set disable_nvm_defaults to non-zero if you want to disable the defaulting behavior : so you can use zero for a value. */
/* […]
Done
https://review.coreboot.org/c/coreboot/+/42466/1/src/drivers/intel/mipi_came... PS1, Line 101:
nit: skip the blank line after the comment, and add one before it starts
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42466/3/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42466/3/src/drivers/intel/mipi_came... PS3, Line 148: case INTEL_ACPI_CAMERA_IMGU: : case INTEL_ACPI_CAMERA_PMIC: just leave these out, default covers it, and it looks like they're still empty at the last patchset
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42466
to look at the new patch set (#4).
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
drivers/intel/mipi_camera: Handle NVM and VCM device type
This change adds support in mipi_camera driver to handle NVM and VCM device types.
Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 64 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/42466/4
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42466/3/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42466/3/src/drivers/intel/mipi_came... PS3, Line 148: case INTEL_ACPI_CAMERA_IMGU: : case INTEL_ACPI_CAMERA_PMIC:
just leave these out, default covers it, and it looks like they're still empty at the last patchset
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 4: Code-Review+2
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
Patch Set 12: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42466 )
Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ......................................................................
drivers/intel/mipi_camera: Handle NVM and VCM device type
This change adds support in mipi_camera driver to handle NVM and VCM device types.
Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42466 Reviewed-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 64 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Rizwan Qureshi: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 4356b0a..b1075b8 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,6 +10,46 @@ #include <device/pci_def.h> #include "chip.h"
+static void camera_fill_nvm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd = acpi_dp_new_table("_DSD"); + + /* It might be possible to default size or width based on type. */ + if (!config->disable_nvm_defaults && !config->nvm_pagesize) + config->nvm_pagesize = 1; + + if (!config->disable_nvm_defaults && !config->nvm_readonly) + config->nvm_readonly = 1; + + if (config->nvm_size) + acpi_dp_add_integer(dsd, "size", config->nvm_size); + + if (config->nvm_pagesize) + acpi_dp_add_integer(dsd, "pagesize", config->nvm_pagesize); + + if (config->nvm_readonly) + acpi_dp_add_integer(dsd, "read-only", config->nvm_readonly); + + if (config->nvm_width) + acpi_dp_add_integer(dsd, "address-width", config->nvm_width); + + acpi_dp_write(dsd); +} + +static void camera_fill_vcm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd; + + if (!config->vcm_compat) + return; + + dsd = acpi_dp_new_table("_DSD"); + acpi_dp_add_string(dsd, "compatible", config->vcm_compat); + acpi_dp_write(dsd); +} + static void write_pci_camera_device(const struct device *dev) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -93,6 +133,17 @@ config->device_type == INTEL_ACPI_CAMERA_VCM) { acpigen_write_name_integer("CAMD", config->device_type); } + + switch (config->device_type) { + case INTEL_ACPI_CAMERA_VCM: + camera_fill_vcm(dev); + break; + case INTEL_ACPI_CAMERA_NVM: + camera_fill_nvm(dev); + break; + default: + break; + } }
static void camera_fill_ssdt(const struct device *dev) diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index c029570..b04a100 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -95,6 +95,19 @@ const char *vcm_name; /* defaults to |vcm_address| device */ uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ uint16_t vcm_address; /* I2C to use if ssdb.vcm_type != 0 */ + /* + * Settings specific to nvram. Many values, if left as zero, will be assigned a default. + * Set disable_nvm_defaults to non-zero if you want to disable the defaulting behavior + * so you can use zero for a value. + */ + bool disable_nvm_defaults; + uint32_t nvm_size; + uint32_t nvm_pagesize; + uint32_t nvm_readonly; + uint32_t nvm_width; + + /* Settings specific to vcm */ + const char *vcm_compat; };
#endif
-
Patrick Georgi (Code Review) -
Rizwan Qureshi (Code Review)
-
Sugnan Prabhu S (Code Review)
-
Tim Wawrzynczak (Code Review)