| Message ID | 1417100881-1523-3-git-send-email-sjoerd.simons@collabora.co.uk |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
On 27 November 2014 at 08:08, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote: > Not all exynos 5420 based devices with an LCD also have a parade LVDS > bridge. So make sure compilation doesn't break if CONFIG_LCD is enabled > and CONFIG_VIDEO_PARADE is not. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > Changes since V1: New patch > > arch/arm/include/asm/arch-exynos/system.h | 4 ++++ > 1 file changed, 4 insertions(+) Acked-by: Simon Glass <sjg@chromium.org> Tested on snow, pit, pi (display does not yet work on Pi): Tested-by: Simon Glass <sjg@chromium.org>
Dear Sjoerd Simons, On 28/11/14 00:08, Sjoerd Simons wrote: > Not all exynos 5420 based devices with an LCD also have a parade LVDS > bridge. So make sure compilation doesn't break if CONFIG_LCD is enabled > and CONFIG_VIDEO_PARADE is not. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > Changes since V1: New patch > > arch/arm/include/asm/arch-exynos/system.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/include/asm/arch-exynos/system.h b/arch/arm/include/asm/arch-exynos/system.h > index 320763f..dd68290 100644 > --- a/arch/arm/include/asm/arch-exynos/system.h > +++ b/arch/arm/include/asm/arch-exynos/system.h > @@ -42,6 +42,10 @@ void set_system_display_ctrl(void); > int exynos_lcd_early_init(const void *blob); > > /* Initialize the Parade dP<->LVDS bridge if present */ > +#ifdef CONFIG_VIDEO_PARADE > int parade_init(const void *blob); > +#else > +static inline int parade_init(const void *blob) { return -1; } > +#endif Actually, it does not related with this patch.. and I know that you are not an author. But.. I'd like ask you, why parade_init function is in exynos header file? If you are agreed, could you please make new header file? (e.g: include/parade.h) And I think you missed removing the CONFIG_VIDEO_PARADE at peach-pi.h Thanks, Minkyu Kang.
Hey Minkyu, On Mon, 2014-12-01 at 14:24 +0900, Minkyu Kang wrote: > > --- a/arch/arm/include/asm/arch-exynos/system.h > > +++ b/arch/arm/include/asm/arch-exynos/system.h > > @@ -42,6 +42,10 @@ void set_system_display_ctrl(void); > > int exynos_lcd_early_init(const void *blob); > > > > /* Initialize the Parade dP<->LVDS bridge if present */ > > +#ifdef CONFIG_VIDEO_PARADE > > int parade_init(const void *blob); > > +#else > > +static inline int parade_init(const void *blob) { return -1; } > > +#endif > > Actually, it does not related with this patch.. > and I know that you are not an author. > But.. I'd like ask you, why parade_init function is in exynos header file? > If you are agreed, could you please make new header file? (e.g: include/parade.h) I'm not sure why it's in the exynos header file, it being there surprised me as well. I'm happy to move it around in a next version of the patch though. > And I think you missed removing the CONFIG_VIDEO_PARADE at peach-pi.h That's on purpose, in the discussion with Simon Glass it became clear that the intention is to start minimizing peach-pi.h (and hopefully make it unnecessary). So i left peach-pi.h untouched for now, in order for a later patchset to clean up the configuration more.
Hi, On 1 December 2014 at 03:06, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote: > Hey Minkyu, > > On Mon, 2014-12-01 at 14:24 +0900, Minkyu Kang wrote: >> > --- a/arch/arm/include/asm/arch-exynos/system.h >> > +++ b/arch/arm/include/asm/arch-exynos/system.h >> > @@ -42,6 +42,10 @@ void set_system_display_ctrl(void); >> > int exynos_lcd_early_init(const void *blob); >> > >> > /* Initialize the Parade dP<->LVDS bridge if present */ >> > +#ifdef CONFIG_VIDEO_PARADE >> > int parade_init(const void *blob); >> > +#else >> > +static inline int parade_init(const void *blob) { return -1; } >> > +#endif >> >> Actually, it does not related with this patch.. >> and I know that you are not an author. >> But.. I'd like ask you, why parade_init function is in exynos header file? >> If you are agreed, could you please make new header file? (e.g: include/parade.h) > > I'm not sure why it's in the exynos header file, it being there > surprised me as well. I'm happy to move it around in a next version of > the patch though. > >> And I think you missed removing the CONFIG_VIDEO_PARADE at peach-pi.h > > That's on purpose, in the discussion with Simon Glass it became clear > that the intention is to start minimizing peach-pi.h (and hopefully make > it unnecessary). So i left peach-pi.h untouched for now, in order for a > later patchset to clean up the configuration more. I'll take a look at this next week. Particularly for Pit and Pi we should be able to use the same config (SDRAM is the only barrier). Regards, Simon
diff --git a/arch/arm/include/asm/arch-exynos/system.h b/arch/arm/include/asm/arch-exynos/system.h index 320763f..dd68290 100644 --- a/arch/arm/include/asm/arch-exynos/system.h +++ b/arch/arm/include/asm/arch-exynos/system.h @@ -42,6 +42,10 @@ void set_system_display_ctrl(void); int exynos_lcd_early_init(const void *blob); /* Initialize the Parade dP<->LVDS bridge if present */ +#ifdef CONFIG_VIDEO_PARADE int parade_init(const void *blob); +#else +static inline int parade_init(const void *blob) { return -1; } +#endif #endif /* _EXYNOS4_SYSTEM_H */
Not all exynos 5420 based devices with an LCD also have a parade LVDS bridge. So make sure compilation doesn't break if CONFIG_LCD is enabled and CONFIG_VIDEO_PARADE is not. Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> --- Changes since V1: New patch arch/arm/include/asm/arch-exynos/system.h | 4 ++++ 1 file changed, 4 insertions(+)