Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sync OP-TEE driver with new naming of interrupts #39

Merged
merged 1 commit into from
Apr 24, 2017
Merged

Sync OP-TEE driver with new naming of interrupts #39

merged 1 commit into from
Apr 24, 2017

Conversation

david-wang-2015
Copy link

In the latest changes of optee_os, the interrupts' names are changed to "native" and "foreign" interrupts.

@david-wang-2015 david-wang-2015 mentioned this pull request Feb 28, 2017
Merged
jenswi-linaro
jenswi-linaro reviewed Feb 28, 2017
View reviewed changes
drivers/tee/optee/optee_smc.h Outdated
@@ -383,19 +383,19 @@ struct optee_smc_disable_shm_cache_result {
OPTEE_SMC_RPC_VAL(OPTEE_SMC_RPC_FUNC_FREE)

/*
* Deliver an IRQ in normal world.
* Deliver a foreign interrupt in normal world.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't "Deliver a foreign interrupt in normal world" sound better?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't "Deliver a foreign interrupt in normal world" sound better?

Hi @jenswi-linaro ,
Sorry, did you suggest to reword the comment "Deliver a foreign interrupt in normal world."

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, paste error. It should have been "Deliver foreign interrupt to normal world".
Once we're in normal world it's not a foreign interrupt anymore.
I'm only suggesting to think about it, if you agree please change otherwise keep it as it is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. :)

@jenswi-linaro
Copy link

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

@david-wang-2015
Sync OP-TEE driver with new naming of interrupts
0d890ff 
In the latest changes of optee_os, the interrupts' names are
changed to "native" and "foreign" interrupts.

Signed-off-by: David Wang <david.wang@arm.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
@david-wang-2015
Copy link
Author

Fixed

@david-wang-2015 david-wang-2015 mentioned this pull request Apr 24, 2017
Merged
@jforissier jforissier merged this pull request into linaro-swg:optee Apr 24, 2017
@jforissier
Copy link 

drivers/tee/optee/rpc.c:377:7: error: 'OPTEE_SMC_RPC_FUNC_IRQ' undeclared (first use in this function)
  case OPTEE_SMC_RPC_FUNC_IRQ:
       ^~~~~~~~~~~~~~~~~~~~~~
drivers/tee/optee/rpc.c:377:7: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:293: recipe for target 'drivers/tee/optee/rpc.o' failed
make[5]: *** [drivers/tee/optee/rpc.o] Error 1

I have reverted this patch (force pushed). @davwan01 have you actually built the driver with your patch applied?

@david-wang-2015
Copy link
Author

Hi @jforissier,
Is it already covered by CI? I don't know how to build and test this repository.

@jforissier
Copy link

jforissier commented Apr 24, 2017 •
edited
Loading

So you don't know how to build linux, yet you are submitting a patch here? 😕
Anyway, it's OK, I can fix the patch myself and merge it. And, no, there is no CI in place on this repo.

@jforissier
Copy link

49ece5d merged.

@david-wang-2015
Copy link
Author

So you don't know how to build linux, yet you are submitting a patch here?
Anyway, it's OK, I can fix the patch myself and merge it. And, no, there is no CI in place on this repo.

Hi @jforissier ,
Sorry, I was asked to sync up the renamed macros in this repository when I was doing the changes in optee_os (OP-TEE/optee_os#1345). Didn't notice it's not covered by CI (at least build)...
BTW, could you point me that combinations of builds/tests I should do locally before PR? Just in case I need to push some patches again.
Thanks for your help. :)

@jbech-linaro
Copy link

@davwan01 , in your case I suspect you're using the Juno board and then I'd suggest that you try it out according to the instructions for Juno here and here.

@david-wang-2015
Copy link
Author

@jbech-linaro , OK, it seems "replace the Linux folder in Linaro release package and test again". :)
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenswi-linaro jenswi-linaro jenswi-linaro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@david-wang-2015 @jenswi-linaro @jforissier @jbech-linaro