diff mbox

ARM: OMAP2+: Prevent redefinition errors for soc.h

Message ID 1352477961-7634-1-git-send-email-jon-hunter@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hunter, Jon Nov. 9, 2012, 4:19 p.m. UTC
If the header soc.h is included more than once in a source (for example
once directly by the source file and once indirectly by another header
file), then the compiler will generate redefintion errors for the macros
in soc.h. Prevent this by only allowing the content in soc.h to be
included once.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---

Please note that I ran into this problem when rebasing my dmtimer fixes
series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
in dmtimer.h and I found several other files including dmtimer.h are also
including soc.h and so generate a lot of errors.

[1] http://marc.info/?l=linux-omap&m=135231490218361&w=2

 arch/arm/mach-omap2/soc.h |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Tony Lindgren Nov. 9, 2012, 4:22 p.m. UTC | #1
* Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
> If the header soc.h is included more than once in a source (for example
> once directly by the source file and once indirectly by another header
> file), then the compiler will generate redefintion errors for the macros
> in soc.h. Prevent this by only allowing the content in soc.h to be
> included once.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
> 
> Please note that I ran into this problem when rebasing my dmtimer fixes
> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
> in dmtimer.h and I found several other files including dmtimer.h are also
> including soc.h and so generate a lot of errors.
> 
> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2

As these headers are private to mach-omap2, I'd rather not allow including
them more than once so we can eventually clean up the includes further.

We should include the headers directly where used, except for the
legacy board-*.c files that will be going away anyways.

Including the files directly should fix this easily, if not let me
know.

Regards,

Tony
 
>  arch/arm/mach-omap2/soc.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
> index 0700964..6b270a0 100644
> --- a/arch/arm/mach-omap2/soc.h
> +++ b/arch/arm/mach-omap2/soc.h
> @@ -25,6 +25,9 @@
>   *
>   */
>  
> +#ifndef __ARCH_ARM_MACH_OMAP2_SOC_H
> +#define __ARCH_ARM_MACH_OMAP2_SOC_H
> +
>  #include "omap24xx.h"
>  #include "omap34xx.h"
>  #include "omap44xx.h"
> @@ -471,3 +474,4 @@ OMAP4_HAS_FEATURE(mpu_1_5ghz, MPU_1_5GHZ)
>  
>  #endif	/* __ASSEMBLY__ */
>  
> +#endif	/* __ARCH_ARM_MACH_OMAP2_SOC_H */
> -- 
> 1.7.9.5
>
Hunter, Jon Nov. 9, 2012, 4:29 p.m. UTC | #2
On 11/09/2012 10:22 AM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
>> If the header soc.h is included more than once in a source (for example
>> once directly by the source file and once indirectly by another header
>> file), then the compiler will generate redefintion errors for the macros
>> in soc.h. Prevent this by only allowing the content in soc.h to be
>> included once.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>
>> Please note that I ran into this problem when rebasing my dmtimer fixes
>> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
>> in dmtimer.h and I found several other files including dmtimer.h are also
>> including soc.h and so generate a lot of errors.
>>
>> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2
> 
> As these headers are private to mach-omap2, I'd rather not allow including
> them more than once so we can eventually clean up the includes further.
> 
> We should include the headers directly where used, except for the
> legacy board-*.c files that will be going away anyways.
> 
> Including the files directly should fix this easily, if not let me
> know.

The alternative fix is to ensure anyone including dmtimer.h also
includes soc.h. However, I did not know if we should have such a
dependency. If you are ok with that then that is what I will do for now.
It is not a massive change.

Cheers
Jon
Tony Lindgren Nov. 9, 2012, 9:10 p.m. UTC | #3
* Jon Hunter <jon-hunter@ti.com> [121109 08:31]:
> 
> On 11/09/2012 10:22 AM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
> >> If the header soc.h is included more than once in a source (for example
> >> once directly by the source file and once indirectly by another header
> >> file), then the compiler will generate redefintion errors for the macros
> >> in soc.h. Prevent this by only allowing the content in soc.h to be
> >> included once.
> >>
> >> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >> ---
> >>
> >> Please note that I ran into this problem when rebasing my dmtimer fixes
> >> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
> >> in dmtimer.h and I found several other files including dmtimer.h are also
> >> including soc.h and so generate a lot of errors.
> >>
> >> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2
> > 
> > As these headers are private to mach-omap2, I'd rather not allow including
> > them more than once so we can eventually clean up the includes further.
> > 
> > We should include the headers directly where used, except for the
> > legacy board-*.c files that will be going away anyways.
> > 
> > Including the files directly should fix this easily, if not let me
> > know.
> 
> The alternative fix is to ensure anyone including dmtimer.h also
> includes soc.h. However, I did not know if we should have such a
> dependency. If you are ok with that then that is what I will do for now.
> It is not a massive change.

Do you mean anything under mach-omap2/*.c including dmtimer.h also
needs to also include soc.h? If sounds OK to me as long as we don't
need to include soc.h outside mach-omap2.

Regards,

Tony
Hunter, Jon Nov. 9, 2012, 11:42 p.m. UTC | #4
On 11/09/2012 03:10 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121109 08:31]:
>>
>> On 11/09/2012 10:22 AM, Tony Lindgren wrote:
>>> * Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
>>>> If the header soc.h is included more than once in a source (for example
>>>> once directly by the source file and once indirectly by another header
>>>> file), then the compiler will generate redefintion errors for the macros
>>>> in soc.h. Prevent this by only allowing the content in soc.h to be
>>>> included once.
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>> ---
>>>>
>>>> Please note that I ran into this problem when rebasing my dmtimer fixes
>>>> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
>>>> in dmtimer.h and I found several other files including dmtimer.h are also
>>>> including soc.h and so generate a lot of errors.
>>>>
>>>> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2
>>>
>>> As these headers are private to mach-omap2, I'd rather not allow including
>>> them more than once so we can eventually clean up the includes further.
>>>
>>> We should include the headers directly where used, except for the
>>> legacy board-*.c files that will be going away anyways.
>>>
>>> Including the files directly should fix this easily, if not let me
>>> know.
>>
>> The alternative fix is to ensure anyone including dmtimer.h also
>> includes soc.h. However, I did not know if we should have such a
>> dependency. If you are ok with that then that is what I will do for now.
>> It is not a massive change.
> 
> Do you mean anything under mach-omap2/*.c including dmtimer.h also
> needs to also include soc.h? If sounds OK to me as long as we don't
> need to include soc.h outside mach-omap2.

Yes exactly. Right now dmtimer.h is including plat/cpu.h and so is
indirectly including soc.h. The function omap_dm_timer_populate_errata()
is using the cpu_is_xxxx macros. So maybe I should move this to into
platform data. Is that better?

Cheers
Jon
Tony Lindgren Nov. 9, 2012, 11:50 p.m. UTC | #5
* Jon Hunter <jon-hunter@ti.com> [121109 15:44]:
> 
> On 11/09/2012 03:10 PM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [121109 08:31]:
> >>
> >> On 11/09/2012 10:22 AM, Tony Lindgren wrote:
> >>> * Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
> >>>> If the header soc.h is included more than once in a source (for example
> >>>> once directly by the source file and once indirectly by another header
> >>>> file), then the compiler will generate redefintion errors for the macros
> >>>> in soc.h. Prevent this by only allowing the content in soc.h to be
> >>>> included once.
> >>>>
> >>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >>>> ---
> >>>>
> >>>> Please note that I ran into this problem when rebasing my dmtimer fixes
> >>>> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
> >>>> in dmtimer.h and I found several other files including dmtimer.h are also
> >>>> including soc.h and so generate a lot of errors.
> >>>>
> >>>> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2
> >>>
> >>> As these headers are private to mach-omap2, I'd rather not allow including
> >>> them more than once so we can eventually clean up the includes further.
> >>>
> >>> We should include the headers directly where used, except for the
> >>> legacy board-*.c files that will be going away anyways.
> >>>
> >>> Including the files directly should fix this easily, if not let me
> >>> know.
> >>
> >> The alternative fix is to ensure anyone including dmtimer.h also
> >> includes soc.h. However, I did not know if we should have such a
> >> dependency. If you are ok with that then that is what I will do for now.
> >> It is not a massive change.
> > 
> > Do you mean anything under mach-omap2/*.c including dmtimer.h also
> > needs to also include soc.h? If sounds OK to me as long as we don't
> > need to include soc.h outside mach-omap2.
> 
> Yes exactly. Right now dmtimer.h is including plat/cpu.h and so is
> indirectly including soc.h. The function omap_dm_timer_populate_errata()
> is using the cpu_is_xxxx macros. So maybe I should move this to into
> platform data. Is that better?

I think I already fixed that up in the cleanup branch. I've now
merged that into the omap-for-v3.8/dt branch. The only remaining
cpu_is_omap usage there is:

$ grep cpu_is_omap arch/arm/plat-omap/*.[chS]

arch/arm/plat-omap/fb.c:        if (cpu_is_omap24xx()) {
arch/arm/plat-omap/fb.c:        } else if (cpu_is_omap34xx()) {

So maybe see what needs to be patches on top of omap-for-v3.8/dt?

Regards,

Tony
Hunter, Jon Nov. 9, 2012, 11:57 p.m. UTC | #6
On 11/09/2012 05:50 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121109 15:44]:
>>
>> On 11/09/2012 03:10 PM, Tony Lindgren wrote:
>>> * Jon Hunter <jon-hunter@ti.com> [121109 08:31]:
>>>>
>>>> On 11/09/2012 10:22 AM, Tony Lindgren wrote:
>>>>> * Jon Hunter <jon-hunter@ti.com> [121109 08:21]:
>>>>>> If the header soc.h is included more than once in a source (for example
>>>>>> once directly by the source file and once indirectly by another header
>>>>>> file), then the compiler will generate redefintion errors for the macros
>>>>>> in soc.h. Prevent this by only allowing the content in soc.h to be
>>>>>> included once.
>>>>>>
>>>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>>>> ---
>>>>>>
>>>>>> Please note that I ran into this problem when rebasing my dmtimer fixes
>>>>>> series [1] on Tony's Linux-OMAP master branch. I am including plat/cpu.h
>>>>>> in dmtimer.h and I found several other files including dmtimer.h are also
>>>>>> including soc.h and so generate a lot of errors.
>>>>>>
>>>>>> [1] http://marc.info/?l=linux-omap&m=135231490218361&w=2
>>>>>
>>>>> As these headers are private to mach-omap2, I'd rather not allow including
>>>>> them more than once so we can eventually clean up the includes further.
>>>>>
>>>>> We should include the headers directly where used, except for the
>>>>> legacy board-*.c files that will be going away anyways.
>>>>>
>>>>> Including the files directly should fix this easily, if not let me
>>>>> know.
>>>>
>>>> The alternative fix is to ensure anyone including dmtimer.h also
>>>> includes soc.h. However, I did not know if we should have such a
>>>> dependency. If you are ok with that then that is what I will do for now.
>>>> It is not a massive change.
>>>
>>> Do you mean anything under mach-omap2/*.c including dmtimer.h also
>>> needs to also include soc.h? If sounds OK to me as long as we don't
>>> need to include soc.h outside mach-omap2.
>>
>> Yes exactly. Right now dmtimer.h is including plat/cpu.h and so is
>> indirectly including soc.h. The function omap_dm_timer_populate_errata()
>> is using the cpu_is_xxxx macros. So maybe I should move this to into
>> platform data. Is that better?
> 
> I think I already fixed that up in the cleanup branch. I've now
> merged that into the omap-for-v3.8/dt branch. The only remaining
> cpu_is_omap usage there is:
> 
> $ grep cpu_is_omap arch/arm/plat-omap/*.[chS]
> 
> arch/arm/plat-omap/fb.c:        if (cpu_is_omap24xx()) {
> arch/arm/plat-omap/fb.c:        } else if (cpu_is_omap34xx()) {
> 
> So maybe see what needs to be patches on top of omap-for-v3.8/dt?

Sorry, I meant in the fixes series I have posted for dmtimer, I am
adding a new function called omap_dm_timer_populate_errata() that is
using cpu_is_xxxx() [1]. I had done this a while back, but now I see
that we want to get away from doing that right?

I was planning on sending you a pull request for that series on Monday
but now I am wondering if I should fix this now or later. I was hoping
this series would make 3.8.

Cheers
Jon

[1] http://marc.info/?l=linux-omap&m=135249008128365&w=2
Tony Lindgren Nov. 10, 2012, 12:21 a.m. UTC | #7
* Jon Hunter <jon-hunter@ti.com> [121109 15:59]:
> 
> Sorry, I meant in the fixes series I have posted for dmtimer, I am
> adding a new function called omap_dm_timer_populate_errata() that is
> using cpu_is_xxxx() [1]. I had done this a while back, but now I see
> that we want to get away from doing that right?

Yes. Looks like everything we have under plat-omap can be moved
to live under drivers eventually, so let's not use cpu_is_omap
there either. For legacy boot, you can use pdata->timer_capability
for passing that?
 
> I was planning on sending you a pull request for that series on Monday
> but now I am wondering if I should fix this now or later. I was hoping
> this series would make 3.8.

Yeah, sounds like that should be fixed though as we still have a bit
of time early next week.

Regards,

Tony
 
> [1] http://marc.info/?l=linux-omap&m=135249008128365&w=2
Hunter, Jon Nov. 10, 2012, 2:52 a.m. UTC | #8
On 11/09/2012 06:21 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121109 15:59]:
>>
>> Sorry, I meant in the fixes series I have posted for dmtimer, I am
>> adding a new function called omap_dm_timer_populate_errata() that is
>> using cpu_is_xxxx() [1]. I had done this a while back, but now I see
>> that we want to get away from doing that right?
> 
> Yes. Looks like everything we have under plat-omap can be moved
> to live under drivers eventually, so let's not use cpu_is_omap
> there either. For legacy boot, you can use pdata->timer_capability
> for passing that?

Yes, but I prefer to keep capability and errata separate. So I have
added an timer_errata variable to the pdata for now.

>> I was planning on sending you a pull request for that series on Monday
>> but now I am wondering if I should fix this now or later. I was hoping
>> this series would make 3.8.
> 
> Yeah, sounds like that should be fixed though as we still have a bit
> of time early next week.

Ok, great. I have re-worked and will stress test over the weekend. I
will send out an updated series Monday and then send a pull request
later in the week if you are ok with it.

Cheers
Jon
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index 0700964..6b270a0 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -25,6 +25,9 @@ 
  *
  */
 
+#ifndef __ARCH_ARM_MACH_OMAP2_SOC_H
+#define __ARCH_ARM_MACH_OMAP2_SOC_H
+
 #include "omap24xx.h"
 #include "omap34xx.h"
 #include "omap44xx.h"
@@ -471,3 +474,4 @@  OMAP4_HAS_FEATURE(mpu_1_5ghz, MPU_1_5GHZ)
 
 #endif	/* __ASSEMBLY__ */
 
+#endif	/* __ARCH_ARM_MACH_OMAP2_SOC_H */