accessing vfpinstr macros from outside vfp directory
(Resending: forgot to add Russell to To: list; subject change to avoid it getting lost in code review thread, was Re: [PATCH v4 2/3] [ARM] perfevents: Add support for Scorpion performance monitors)
Russell,
Quoted 37 lines · Jean Pihet
Quoted 18 lines · Sheetal Sahasrabudhe
Hi Will/Jean,
Quoted 15 lines · Bryan Huntsman
Quoted 13 lines · Sheetal Sahasrabudhe
This commit adds support for performance monitors provided by Qualcomm Scorpion and ScorpionMP processor to perfevents.
Signed-off-by Sheetal Sahasrabudhe<[email protected]>Reviewed-by Jean Pihet<[email protected]>Reviewed-by Will Deacon<[email protected]>--- arch/arm/include/asm/perf_event.h | 2 + arch/arm/kernel/perf_event.c | 11 + arch/arm/kernel/perf_event_msm.c | 679 +++++++++++++++++++++++++++++++++++++ 3 files changed, 692 insertions(+), 0 deletions(-)create mode 100644
arch/arm/kernel/perf_event_msm.cQuoted 17 lines
Quoted 7 lines
...
Quoted 5 lines
arch/arm/kernel/perf_event_msm.c new file +0
-0[...]
Quoted 8 lines
arch/arm/kernel/perf_event_msm.c
Quoted 3 lines
arch/arm/kernel/perf_event_msm.c new file +3
-0+#include <asm/vfp.h> +#include <asm/system.h> +#include "../vfp/vfpinstr.h" Sorry I didn't see this earlier. Is there another way to get the info you need that wouldn't use a relative include path? If the info from
vfpinstr.his now needed outside of the vfp directory, can it be moved to a common header instead? Thanks.
Quoted 7 lines
Good catch! Sorry I did not catch this one while reviewing the code.
Quoted 5 lines
Quoted 2 lines
- Bryan
I see other files under vfp that include this header.
So if we were to implement Bryan's suggestion, I think a separate patch that moves the
header and updates all the depdendencies would be more appropriate.
Quoted 1 line
Quoted 1 line
Are you okay with the relative include path for
vfpinstr.hin this patch for now or
would you want to see an update to move vfpinstr.h to include/asm?.
Quoted 10 lines
Adding Russell and Nicolas for the VFP question.
Only fmrx and fmxr are used from "../vfp/vfpinstr.h". Can those be moved toarch/arm/include/asm/vfp.h?What is the best solution to this?
Regards,
Jean
Any opinions on what would be the best thing to do here? Choices appear to be:
1) allow the relative include path of ../vfp/vfpinstr.h
2) move the definitions of fmrx, fmxr from vfp/vfpinstr.h to include/asm/vfp.h
3) move vfp/vfpinstr.h to include/asm
4) other...?
If it helps, I can create a patch for whichever is considered the preferred solution.
Thanks,
Neil (Standing in temporarily for Sheetal while she is on leave of absence)
Quoted 4 lines
Quoted 4 lines
Thanks,
Sheetal
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum
Neil,
Quoted 9 lines · Neil Leeder
Any opinions on what would be the best thing to do here? Choices appear to be:
1) allow the relative include path of
../vfp/vfpinstr.h
2) move the definitions of fmrx, fmxr fromvfp/vfpinstr.htoinclude/asm/vfp.h
3) movevfp/vfpinstr.htoinclude/asm
4) other...?If it helps, I can create a patch for whichever is considered the preferred solution.
I personally don't find option (1) that offensive - Bryan seemed to differ though so perhaps option (2) would keep him happy?
I don't think option (3) is sensible given that the majority of the header file is private to /vfp.
Will
Quoted 13 lines · Will Deacon
Neil,
Quoted 9 lines · Neil Leeder
Any opinions on what would be the best thing to do here? Choices appear to be:
1) allow the relative include path of
../vfp/vfpinstr.h
2) move the definitions of fmrx, fmxr fromvfp/vfpinstr.htoinclude/asm/vfp.h
3) movevfp/vfpinstr.htoinclude/asm
4) other...?If it helps, I can create a patch for whichever is considered the preferred solution.
Quoted 7 lines
I personally don't find option (1) that offensive - Bryan seemed to differ though so perhaps option (2) would keep him happy?
I don't think option (3) is sensible given that the majority of the header file is private to /vfp.
Will
I raised it as a question. If that's the only sane thing to do here, then do it. Since no one else seems to have chimed in on (2) or (3), I'm fine with the patch as-is.
- Bryan
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Hi Bryan,
Quoted 20 lines · Bryan Huntsman
Quoted 4 lines · Will Deacon
Neil,
Quoted 1 line · Neil Leeder
Any opinions on what would be the best thing to do here? Choices
appear to be:
Quoted 3 lines
Quoted 3 lines
1) allow the relative include path of
../vfp/vfpinstr.h
2) move the definitions of fmrx, fmxr fromvfp/vfpinstr.hto
include/asm/vfp.hQuoted 4 lines
Quoted 4 lines
3) move
vfp/vfpinstr.htoinclude/asm
4) other...?If it helps, I can create a patch for whichever is considered the
preferred solution.
Quoted 5 lines
Quoted 1 line
I personally don't find option (1) that offensive - Bryan seemed to differ though so perhaps option (2) would keep him happy?
Quoted 8 lines
Quoted 4 lines
I don't think option (3) is sensible given that the majority of the header file is private to /vfp.
Will
I raised it as a question. If that's the only sane thing to do here, then do it. Since no one else seems to have chimed in on (2) or (3), I'm fine with the patch as-is.
Ok, great. Let's leave it like it is for the time-being and if other people start using things from the vfp headers then we should consider refactoring some of that code (currently I think it's restricted to this patch series and some files under vfp/).
Cheers,
Will
Will and Bryan,
Quoted 28 lines · Will Deacon
Hi Bryan,
Quoted 25 lines · Bryan Huntsman
Quoted 4 lines · Will Deacon
Neil,
Quoted 1 line · Neil Leeder
Any opinions on what would be the best thing to do here? Choices
appear to be:
Quoted 3 lines
Quoted 3 lines
1) allow the relative include path of
../vfp/vfpinstr.h
2) move the definitions of fmrx, fmxr fromvfp/vfpinstr.hto
include/asm/vfp.hQuoted 4 lines
Quoted 4 lines
3) move
vfp/vfpinstr.htoinclude/asm
4) other...?If it helps, I can create a patch for whichever is considered the
preferred solution.
Quoted 9 lines
Quoted 1 line
I personally don't find option (1) that offensive - Bryan seemed to differ though so perhaps option (2) would keep him happy?
I don't think option (3) is sensible given that the majority of the header file is private to /vfp.
Will
Quoted 8 lines
Quoted 3 lines
I raised it as a question. If that's the only sane thing to do here, then do it. Since no one else seems to have chimed in on (2) or (3), I'm fine with the patch as-is.
Ok, great. Let's leave it like it is for the time-being and if other people start using things from the vfp headers then we should consider refactoring some of that code (currently I think it's restricted to this patch series and some files under vfp/).
Sounds good to me - thank you both for the comments.
Neil
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum