[PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[Krzysztof Kozlowski]

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
 
 /* 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 */
-- 
2.9.3

[PATCH 2/2] drm/exynos/decon: Move headers from global to local place

[Krzysztof Kozlowski]

The DECON headers contain only defines for registers.  There are no
other drivers using them so this should be put locally to the Exynos DRM
driver.  Keeping headers local helps managing the code.

Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c                          | 3 +--
 drivers/gpu/drm/exynos/exynos7_drm_decon.c                             | 2 +-
 .../exynos5433_decon.h => drivers/gpu/drm/exynos/regs-decon5433.h      | 0
 include/video/exynos7_decon.h => drivers/gpu/drm/exynos/regs-decon7.h  | 3 +--
 4 files changed, 3 insertions(+), 5 deletions(-)
 rename include/video/exynos5433_decon.h => drivers/gpu/drm/exynos/regs-decon5433.h (100%)
 rename include/video/exynos7_decon.h => drivers/gpu/drm/exynos/regs-decon7.h (99%)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 5792ca88ab7a..5aca8c16f2bc 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -19,13 +19,12 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
-#include <video/exynos5433_decon.h>
-
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
 #include "exynos_drm_fb.h"
 #include "exynos_drm_plane.h"
 #include "exynos_drm_iommu.h"
+#include "regs-decon5433.h"
 
 #define DSD_CFG_MUX 0x1004
 #define DSD_CFG_MUX_TE_UNMASK_GLOBAL BIT(13)
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 3e88269fdc2e..387e436c97d2 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -25,13 +25,13 @@
 
 #include <video/of_display_timing.h>
 #include <video/of_videomode.h>
-#include <video/exynos7_decon.h>
 
 #include "exynos_drm_crtc.h"
 #include "exynos_drm_plane.h"
 #include "exynos_drm_drv.h"
 #include "exynos_drm_fb.h"
 #include "exynos_drm_iommu.h"
+#include "regs-decon7.h"
 
 /*
  * DECON stands for Display and Enhancement controller.
diff --git a/include/video/exynos5433_decon.h b/drivers/gpu/drm/exynos/regs-decon5433.h
similarity index 100%
rename from include/video/exynos5433_decon.h
rename to drivers/gpu/drm/exynos/regs-decon5433.h
diff --git a/include/video/exynos7_decon.h b/drivers/gpu/drm/exynos/regs-decon7.h
similarity index 99%
rename from include/video/exynos7_decon.h
rename to drivers/gpu/drm/exynos/regs-decon7.h
index d28829659a17..8fe4c44f12f3 100644
--- a/include/video/exynos7_decon.h
+++ b/drivers/gpu/drm/exynos/regs-decon7.h
@@ -1,5 +1,4 @@
-/* include/video/exynos7_decon.h
- *
+/*
  * Copyright (c) 2014 Samsung Electronics Co., Ltd.
  * Author: Ajay Kumar <ajaykumar.rs@samsung.com>
  *
-- 
2.9.3

Re: [PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[Emil Velikov]

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

Re: [PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[Krzysztof Kozlowski]

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

Re: [PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[Emil Velikov]

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

Re: [PATCH 1/2] drm/exynos/decon: Add include guard to the Exynos7 header

[Krzysztof Kozlowski]

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