Re: [PATCH v6] omap4-fb: add driver

From: Alexander Aring <alex.aring@gmail.com>
To: Christoph Fritz <chf.fritz@googlemail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v6] omap4-fb: add driver
Date: Mon, 17 Jun 2013 20:45:48 +0200	[thread overview]
Message-ID: <20130617184511.GA1698@x61s.8.8.8.8> (raw)
In-Reply-To: <1371222662.28854.15.camel@mars>

Hi,

only a very small nitpick.

On Fri, Jun 14, 2013 at 05:11:02PM +0200, Christoph Fritz wrote:
> This patch adds omap4 display controller support.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
> changes since v2:
>         - use dev_request_mem_region_by_name()
> changes since v3:
>         - remove register struct
>         - use uncached screen_base
> changes since v4:
>         - remove useless dev_add_param()
>         - use wait_on_timeout() instead while-deadlock
> changes since v5:
> 	- use dev_dbg() and dev_err(), be less verbose
> 	- fix coding-style issues
> 	- add omap4fb_find_display_by_name()
> 	- add additional read/write macros
> ---
>  arch/arm/mach-omap/Makefile                |    1 +
>  arch/arm/mach-omap/include/mach/omap4-fb.h |   46 +++
>  arch/arm/mach-omap/omap4_fb.c              |   27 ++
>  drivers/video/Kconfig                      |    8 +
>  drivers/video/Makefile                     |    1 +
>  drivers/video/omap4.c                      |  526 ++++++++++++++++++++++++++++
>  drivers/video/omap4.h                      |  187 ++++++++++
>  7 files changed, 796 insertions(+)
>  create mode 100644 arch/arm/mach-omap/include/mach/omap4-fb.h
>  create mode 100644 arch/arm/mach-omap/omap4_fb.c
>  create mode 100644 drivers/video/omap4.c
>  create mode 100644 drivers/video/omap4.h
> 
> diff --git a/arch/arm/mach-omap/Makefile b/arch/arm/mach-omap/Makefile
> index 94e42c6..e70ddbd 100644
> --- a/arch/arm/mach-omap/Makefile
> +++ b/arch/arm/mach-omap/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_OMAP3_CLOCK_CONFIG) += omap3_clock.o
>  pbl-$(CONFIG_OMAP3_CLOCK_CONFIG) += omap3_clock.o
>  obj-$(CONFIG_OMAP_GPMC) += gpmc.o devices-gpmc-nand.o
>  obj-$(CONFIG_SHELL_NONE) += xload.o
> +obj-$(CONFIG_DRIVER_VIDEO_OMAP4) += omap4_fb.o
>  obj-$(CONFIG_I2C_TWL6030) += omap4_twl6030_mmc.o
>  obj-$(CONFIG_OMAP4_USBBOOT) += omap4_rom_usb.o
>  obj-y += gpio.o
> diff --git a/arch/arm/mach-omap/include/mach/omap4-fb.h b/arch/arm/mach-omap/include/mach/omap4-fb.h
> new file mode 100644
> index 0000000..5c0a54b
> --- /dev/null
> +++ b/arch/arm/mach-omap/include/mach/omap4-fb.h
> @@ -0,0 +1,46 @@
> +#ifndef H_BAREBOX_ARCH_ARM_MACH_OMAP_MACH_FB4_H
> +#define H_BAREBOX_ARCH_ARM_MACH_OMAP_MACH_FB4_H
> +
> +#include <fb.h>
> +
> +#define OMAP_DSS_LCD_TFT	(1u << 0)
> +#define OMAP_DSS_LCD_IVS	(1u << 1)

...

> +
> +	if (!pdata)
> +		return -ENODEV;
> +

In my opinion this is a false errno for this case. But better is to drop
this check completely. It's only a check that a programmer doesn't set a
pdata which is very unlikely.

I think there was another patches on this list to remove something like
this.

Regards
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox