Attention is currently required from: Jason Glenesk, Raul Rangel, Martin L Roth, Marshall Dawson, Matt DeVillier, Paul Menzel, Arthur Heymans, Fred Reitberger, Felix Held.
ritul guru has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65523 )
Change subject: soc/amd/common/psp: Support PSP SMI handler ......................................................................
Patch Set 24:
(5 comments)
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/0bfbfaac_6b194e01 PS24, Line 119: static bool is_valid_psp_spi_id(u64 id) : { : return id == SMI_TARGET_NVRAM || : id == SMI_TARGET_RPMC_NVRAM; : } : : bool is_valid_psp_spi_info(struct mbox_default_buffer *buffer) : { : struct mbox_pspv2_cmd_spi_info *cmd = (struct mbox_pspv2_cmd_spi_info *)buffer; : return is_valid_psp_spi_id(read64(&cmd->req.id)); : } : : bool is_valid_psp_spi_read_write(struct mbox_default_buffer *buffer) : { : struct mbox_pspv2_cmd_spi_read_write *cmd = : (struct mbox_pspv2_cmd_spi_read_write *)buffer; : return is_valid_psp_spi_id(read64(&cmd->req.id)); : } : : bool is_valid_psp_spi_erase(struct mbox_default_buffer *buffer) : { : struct mbox_pspv2_cmd_spi_erase *cmd = (struct mbox_pspv2_cmd_spi_erase *)buffer; : return is_valid_psp_spi_id(read64(&cmd->req.id)); : } : : u64 get_psp_spi_info_id(struct mbox_default_buffer *buffer) : { : struct mbox_pspv2_cmd_spi_info *cmd = (struct mbox_pspv2_cmd_spi_info *)buffer; : return read64(&cmd->req.id); : } : : void set_psp_spi_info(struct mbox_default_buffer *buffer, : u64 lba, u64 block_size, u64 num_blocks) : { : struct mbox_pspv2_cmd_spi_info *cmd = (struct mbox_pspv2_cmd_spi_info *)buffer; : write32(&cmd->req.lba, lba); : write32(&cmd->req.block_size, block_size); : write32(&cmd->req.num_blocks, num_blocks); : } : : void get_psp_spi_read_write(struct mbox_default_buffer *buffer, : u64 *id, u64 *lba, u64 *offset, : u64 *num_bytes, u8 **data) : { : struct mbox_pspv2_cmd_spi_read_write *cmd = : (struct mbox_pspv2_cmd_spi_read_write *)buffer; : *id = read64(&cmd->req.id); : *lba = read64(&cmd->req.lba); : *offset = read64(&cmd->req.offset); : *num_bytes = read64(&cmd->req.num_bytes); : *data = cmd->req.buffer; : } : : void get_psp_spi_erase(struct mbox_default_buffer *buffer, : u64 *id, u64 *lba, u64 *num_blocks) : { : struct mbox_pspv2_cmd_spi_erase *cmd = (struct mbox_pspv2_cmd_spi_erase *)buffer; : *id = read64(&cmd->req.id); : *lba = read64(&cmd->req.lba); : *num_blocks = read64(&cmd->req.num_blocks); : } :
all these newly added functions only get used in soc/amd/common/block/psp/psp_smi. […]
I do not see a point now again moving back these to psp_p2c.c!
File src/soc/amd/common/block/psp/psp_smi.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/a628ec06_29fd7bc0 PS24, Line 63: int
since this basically returns a bool, i'd change the return type to bool
Ack
https://review.coreboot.org/c/coreboot/+/65523/comment/0741de9b_a95c4a61 PS24, Line 68: const struct spi_flash **flash
only flash->sector_size is used from this struct in all cases, so i'd replace this parameter with u6 […]
prefer the existing way.
https://review.coreboot.org/c/coreboot/+/65523/comment/74b80591_c23738f1 PS24, Line 68: static enum mbox_p2c_status find_psp_spi_flash_device_region(const struct spi_flash **flash)
this function is missing the struct region_device *store parameter and the id parameter
clarify in detail.
https://review.coreboot.org/c/coreboot/+/65523/comment/effd92af_e7bad0db PS24, Line 100: find_spi_flash_device_region
find_psp_spi_flash_device_region
why?