| Message ID | 20170619163110.28173-1-krzk@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19 June 2017 at 17:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: > Although header is included only once but still having an include guard > is a good practice. To avoid confusion, add SoC prefix to existing > Exynos5433 header include guard. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > include/video/exynos5433_decon.h | 6 +++--- > include/video/exynos7_decon.h | 5 +++++ > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h > index 78957c9626f5..b30362da5692 100644 > --- a/include/video/exynos5433_decon.h > +++ b/include/video/exynos5433_decon.h > @@ -6,8 +6,8 @@ > * published by the Free Software Foundationr > */ > > -#ifndef EXYNOS_REGS_DECON_H > -#define EXYNOS_REGS_DECON_H > +#ifndef EXYNOS5433_REGS_DECON_H > +#define EXYNOS5433_REGS_DECON_H > Drop the _REGS_ part from the guard on each header? The file name/path does not have it, plus it'll save some WTF moments when exynos{5433,7}_regs_decon.h comes about. Regards, Emil
On Tue, Jun 20, 2017 at 11:53 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 19 June 2017 at 17:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> Although header is included only once but still having an include guard >> is a good practice. To avoid confusion, add SoC prefix to existing >> Exynos5433 header include guard. >> >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >> --- >> include/video/exynos5433_decon.h | 6 +++--- >> include/video/exynos7_decon.h | 5 +++++ >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h >> index 78957c9626f5..b30362da5692 100644 >> --- a/include/video/exynos5433_decon.h >> +++ b/include/video/exynos5433_decon.h >> @@ -6,8 +6,8 @@ >> * published by the Free Software Foundationr >> */ >> >> -#ifndef EXYNOS_REGS_DECON_H >> -#define EXYNOS_REGS_DECON_H >> +#ifndef EXYNOS5433_REGS_DECON_H >> +#define EXYNOS5433_REGS_DECON_H >> > Drop the _REGS_ part from the guard on each header? The file name/path > does not have it, plus it'll save some WTF moments when > exynos{5433,7}_regs_decon.h comes about. So maybe it makes sense to reorder these patches and use the guard name matching final file name? Best regards, Krzysztof
On 20 June 2017 at 11:02, Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Tue, Jun 20, 2017 at 11:53 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 19 June 2017 at 17:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: >>> Although header is included only once but still having an include guard >>> is a good practice. To avoid confusion, add SoC prefix to existing >>> Exynos5433 header include guard. >>> >>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >>> --- >>> include/video/exynos5433_decon.h | 6 +++--- >>> include/video/exynos7_decon.h | 5 +++++ >>> 2 files changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h >>> index 78957c9626f5..b30362da5692 100644 >>> --- a/include/video/exynos5433_decon.h >>> +++ b/include/video/exynos5433_decon.h >>> @@ -6,8 +6,8 @@ >>> * published by the Free Software Foundationr >>> */ >>> >>> -#ifndef EXYNOS_REGS_DECON_H >>> -#define EXYNOS_REGS_DECON_H >>> +#ifndef EXYNOS5433_REGS_DECON_H >>> +#define EXYNOS5433_REGS_DECON_H >>> >> Drop the _REGS_ part from the guard on each header? The file name/path >> does not have it, plus it'll save some WTF moments when >> exynos{5433,7}_regs_decon.h comes about. > > So maybe it makes sense to reorder these patches and use the guard > name matching final file name? > That sounds better, IMHO. -Emil
On Tue, Jun 20, 2017 at 12:57 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 20 June 2017 at 11:02, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> On Tue, Jun 20, 2017 at 11:53 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 19 June 2017 at 17:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>> Although header is included only once but still having an include guard >>>> is a good practice. To avoid confusion, add SoC prefix to existing >>>> Exynos5433 header include guard. >>>> >>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >>>> --- >>>> include/video/exynos5433_decon.h | 6 +++--- >>>> include/video/exynos7_decon.h | 5 +++++ >>>> 2 files changed, 8 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h >>>> index 78957c9626f5..b30362da5692 100644 >>>> --- a/include/video/exynos5433_decon.h >>>> +++ b/include/video/exynos5433_decon.h >>>> @@ -6,8 +6,8 @@ >>>> * published by the Free Software Foundationr >>>> */ >>>> >>>> -#ifndef EXYNOS_REGS_DECON_H >>>> -#define EXYNOS_REGS_DECON_H >>>> +#ifndef EXYNOS5433_REGS_DECON_H >>>> +#define EXYNOS5433_REGS_DECON_H >>>> >>> Drop the _REGS_ part from the guard on each header? The file name/path >>> does not have it, plus it'll save some WTF moments when >>> exynos{5433,7}_regs_decon.h comes about. >> >> So maybe it makes sense to reorder these patches and use the guard >> name matching final file name? >> > That sounds better, IMHO. OK then, I'll re-order the patches and use matching name (EXYNOS_REGS_DECON{5433,7}_H). Best regards, Krzysztof
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h index 78957c9626f5..b30362da5692 100644 --- a/include/video/exynos5433_decon.h +++ b/include/video/exynos5433_decon.h @@ -6,8 +6,8 @@ * published by the Free Software Foundationr */ -#ifndef EXYNOS_REGS_DECON_H -#define EXYNOS_REGS_DECON_H +#ifndef EXYNOS5433_REGS_DECON_H +#define EXYNOS5433_REGS_DECON_H /* Exynos543X DECON */ #define DECON_VIDCON0 0x0000 @@ -206,4 +206,4 @@ #define CRCCTRL_CRCEN (0x1 << 0) #define CRCCTRL_MASK (0x7) -#endif /* EXYNOS_REGS_DECON_H */ +#endif /* EXYNOS5433_REGS_DECON_H */ diff --git a/include/video/exynos7_decon.h b/include/video/exynos7_decon.h index a62b11b613f6..d28829659a17 100644 --- a/include/video/exynos7_decon.h +++ b/include/video/exynos7_decon.h @@ -9,6 +9,9 @@ * option) any later version. */ +#ifndef EXYNOS7_REGS_DECON_H +#define EXYNOS7_REGS_DECON_H + /* VIDCON0 */ #define VIDCON0 0x00 @@ -347,3 +350,5 @@ #define DECON_UPDATE_SLAVE_SYNC (1 << 4) #define DECON_UPDATE_STANDALONE_F (1 << 0) + +#endif /* EXYNOS7_REGS_DECON_H */
Although header is included only once but still having an include guard is a good practice. To avoid confusion, add SoC prefix to existing Exynos5433 header include guard. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- include/video/exynos5433_decon.h | 6 +++--- include/video/exynos7_decon.h | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-)