[U-Boot] [PATCH v4 2/2] sunxi: video: Add simplefb support

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/18/2014 03:32 PM, Simon Glass wrote:
> Hi Hans,
> 
> 
> On 18 November 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 11/17/2014 07:39 PM, Simon Glass wrote:
>>> Hi Hans,
>>>
>>> On 17 November 2014 15:48, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> From: Luc Verhaegen <libv@skynet.be>
>>>>
>>>> Add simplefb support, note this depends on the kernel having support for
>>>> the clocks property which has recently been added to the simplefb devicetree
>>>> binding.
>>>>
>>>> Signed-off-by: Luc Verhaegen <libv@skynet.be>
>>>> [hdegoede at redhat.com: Use pre-populated simplefb node under /chosen as
>>>>  disussed on the devicetree list]
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>  arch/arm/include/asm/arch-sunxi/display.h |  4 ++++
>>>>  board/sunxi/board.c                       | 11 +++++++++
>>>>  drivers/video/sunxi_display.c             | 39 +++++++++++++++++++++++++++++++
>>>>  include/configs/sunxi-common.h            |  8 +++++++
>>>>  4 files changed, 62 insertions(+)
>>>>
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h
>>>> index 8d80ceb..4c694f8 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/display.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/display.h
>>>> @@ -195,4 +195,8 @@ struct sunxi_hdmi_reg {
>>>>  #define SUNXI_HDMI_PLL_DBG0_PLL3               (0 << 21)
>>>>  #define SUNXI_HDMI_PLL_DBG0_PLL7               (1 << 21)
>>>>
>>>> +#ifdef CONFIG_VIDEO_DT_SIMPLEFB
>>>> +void sunxi_simplefb_setup(void *blob);
>>>> +#endif
>>>> +
>>>>  #endif /* _SUNXI_DISPLAY_H */
>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>> index e6ec5b8..d4530e8 100644
>>>> --- a/board/sunxi/board.c
>>>> +++ b/board/sunxi/board.c
>>>> @@ -24,6 +24,7 @@
>>>>  #endif
>>>>  #include <asm/arch/clock.h>
>>>>  #include <asm/arch/cpu.h>
>>>> +#include <asm/arch/display.h>
>>>>  #include <asm/arch/dram.h>
>>>>  #include <asm/arch/gpio.h>
>>>>  #include <asm/arch/mmc.h>
>>>> @@ -237,3 +238,13 @@ int misc_init_r(void)
>>>>         return 0;
>>>>  }
>>>>  #endif
>>>> +
>>>> +#ifdef CONFIG_OF_BOARD_SETUP
>>>> +void
>>>> +ft_board_setup(void *blob, bd_t *bd)
>>>> +{
>>>> +#ifdef CONFIG_VIDEO_DT_SIMPLEFB
>>>> +       sunxi_simplefb_setup(blob);
>>>> +#endif
>>>> +}
>>>> +#endif /* CONFIG_OF_BOARD_SETUP */
>>>> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
>>>> index 3f46c31..74c4bd3 100644
>>>> --- a/drivers/video/sunxi_display.c
>>>> +++ b/drivers/video/sunxi_display.c
>>>> @@ -13,6 +13,8 @@
>>>>  #include <asm/arch/display.h>
>>>>  #include <asm/global_data.h>
>>>>  #include <asm/io.h>
>>>> +#include <fdtdec.h>
>>>> +#include <fdt_support.h>
>>>>  #include <linux/fb.h>
>>>>  #include <video_fb.h>
>>>>
>>>> @@ -416,3 +418,40 @@ video_hw_init(void)
>>>>
>>>>         return graphic_device;
>>>>  }
>>>> +
>>>> +/*
>>>> + * Simplefb support.
>>>> + */
>>>> +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB)
>>>> +void
>>>> +sunxi_simplefb_setup(void *blob)
>>>> +{
>>>> +       static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
>>>> +       int offset, ret;
>>>> +
>>>> +       if (!sunxi_display.enabled)
>>>> +               return;
>>>
>>> return -ENOENT?
>>
>> If people want this, I can change the proto to an int and make sunxi_simplefb_setup
>> return error codes as you suggest, but this function gets called from
>> ft_board_setup which is void itself, so there is no where to propagate the error,
>> and more-over we do not want simplefb setup errors to be treated as fatal, so
>> I see little use in having it return error codes.
> 
> ft_board_setup() will soon change to return an error. Will likely
> merge those patches next week.

That is good to hear, but not relevant in this case, as said:

"more-over we do not want simplefb setup errors to be treated as fatal"

Main reason for this is that older dtb-s do not have the pre-populated
simplefb node. I guess it could be argued that not having the node
should be a warning (and return 0), and the other errors should be
real errors. I can be convinced to make that change, let me know either
way.

Regards,

Hans