Adsp main remoteproc #2497
Adsp main remoteproc #2497
Conversation
- Renamed variables for a clearer relevance to their function - Reorganizing code for better readibility - introducing early returns where possible Signed-off-by: UtsavAgarwalADI <[email protected]>
UtsavAgarwalADI
force-pushed
the
adsp-main-remoteproc
branch
2 times, most recently
from
fad1681
to
32c964d
Compare
| default: | ||
| dev_err(rproc_data->dev, | ||
| "%s, invalid svect:0x%lx, cord_id:%d\n", | ||
| __func__, svect, core_id); |
There was a problem hiding this comment.
before change core_id was rpoc_data->core_id,
after the change core_id = !!svect && rproc_data->core_id
dev_err core_id will not behave same as before
drivers/remoteproc/adi_remoteproc.c
Outdated
| @@ -211,11 +226,15 @@ static int adi_core_reset(struct adi_rproc_data *rproc_data) | |||
|
|
|||
| static int adi_core_stop(struct adi_rproc_data *rproc_data) | |||
| { | |||
| if (!rproc_data->adi_rsc_table) | |||
There was a problem hiding this comment.
if adi_core_stop is called, thats because the adi_core_start succeeded, so no need double check rpoc_data->adi_rsc_table as that was already done
drivers/remoteproc/adi_remoteproc.c
Outdated
| * to verify the block header via an xor checksum of the bcode_flag field. | ||
| */ | ||
|
|
||
| static int verify_buffer(struct adi_rproc_data *rproc_data, |
There was a problem hiding this comment.
can this be renamed to more specif name, this generic can be adi_verify_blk_header or anything meaningful
drivers/remoteproc/adi_remoteproc.c
Outdated
| uint8_t expected_xor_checksum; | ||
|
|
||
| expected_xor_checksum = block_hdr->bcode_flag.bHdrCHK; | ||
| curr_hdr_xor_checksum = (0xFF000000 & *(uint32_t *)block_hdr) ^ \ |
There was a problem hiding this comment.
doesn't seems right, operating with uint32_t and then doing XOR and making equal to uint8_t seems that upper 3 bytes values will be lost in conversion
| } | ||
|
|
||
| if (rproc_data->rpmsg_state == ADI_RP_RPMSG_SYNCED) | ||
| if (rproc_data->rproc_state == ADI_REMOTEPROC_SYNCED) { | ||
| adi_tru_trigger_device(rproc_data->tru, rproc_data->dev); |
There was a problem hiding this comment.
does second time adi_tru_trigger_device is required, its already done once on line 816
There was a problem hiding this comment.
Thats the case when its already synced, if its not waiting but not synced either, the state is changed again and then tru is triggered. I could maybe add a goto statement for more code reuse, but I think that an extra jump since that would just be substituting the one line function call.
|
|
||
| hdr = &rproc_data->adi_rsc_table->adi_table_hdr; | ||
| core_init = hdr->initialized; | ||
| for (wait_time = 0; wait_time < CORE_INIT_TIMEOUT_MS; wait_time += 20) { |
There was a problem hiding this comment.
Before the logic was to wait if state ADI_RP_RPMSG_WAITING , now logic changed to if not ADI_RP_RPMSG_WAITING state then wait. Is this required?
There was a problem hiding this comment.
Inverting it allows us to return early and avoid indentation. If you see further in the code, the logic for a wait is still preserved
UtsavAgarwalADI
force-pushed
the
adsp-main-remoteproc
branch
from
32c964d
to
3b52143
Compare
- Remoteproc can now verify ldr headers to ensure uncorrupted stream (including using XOR checksum where possible) Signed-off-by: UtsavAgarwalADI <[email protected]>
UtsavAgarwalADI
force-pushed
the
adsp-main-remoteproc
branch
from
c448e95
to
24e123d
Compare
- Fixing core_id for loading svect - Removing extra check for resource table from adi_core_stop - Renaming verification function for a more accurate representation of its working Signed-off-by: UtsavAgarwalADI <[email protected]>
Signed-off-by: UtsavAgarwalADI <[email protected]>
PR Description
Added a verification function for LDR files
Simplifying code and introducing early returns where possible
PR Type
PR Checklist