On Tue, Mar 12, 2013 at 12:19:38PM +0200, Tomi Valkeinen wrote:
> Structs videomode and display_timing have rather long field names for
> the timing values. Nothing wrong with that as such, but this patch
> changes them to abbreviations for the following reasons:
> 
> * The timing values often need to be used in calculations, and long
>   field names makes their direct use clumsier.
> 
> * The current names are a bit of a mishmash: some words are used as
>   such, some are shortened, and for some only first letter is used. Some
>   names use underscode, some don't. All this makes it difficult to
>   remember what the field names are.
> 
> * The abbreviations used in this patch are very common, and there
>   shouldn't be any misunderstanding about their meaning.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> Cc: Steffen Trumtrar <s.trumtrar at pengutronix.de>
> ---
>  drivers/gpu/drm/drm_modes.c       |   18 +++++++++---------
>  drivers/video/fbmon.c             |   24 +++++++++++-------------
>  drivers/video/of_display_timing.c |   16 ++++++++--------
>  drivers/video/videomode.c         |   16 ++++++++--------
>  include/video/display_timing.h    |   16 ++++++++--------
>  include/video/videomode.h         |   18 +++++++++---------
>  6 files changed, 53 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index f83f071..d744e95 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -510,15 +510,15 @@ EXPORT_SYMBOL(drm_gtf_mode);
>  int drm_display_mode_from_videomode(const struct videomode *vm,
>                                   struct drm_display_mode *dmode)
>  {
> -     dmode->hdisplay = vm->hactive;
> -     dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
> -     dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
> -     dmode->htotal = dmode->hsync_end + vm->hback_porch;
> +     dmode->hdisplay = vm->hact;
> +     dmode->hsync_start = dmode->hdisplay + vm->hfp;
> +     dmode->hsync_end = dmode->hsync_start + vm->hsl;
> +     dmode->htotal = dmode->hsync_end + vm->hbp;
>  
> -     dmode->vdisplay = vm->vactive;
> -     dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
> -     dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
> -     dmode->vtotal = dmode->vsync_end + vm->vback_porch;
> +     dmode->vdisplay = vm->vact;
> +     dmode->vsync_start = dmode->vdisplay + vm->vfp;
> +     dmode->vsync_end = dmode->vsync_start + vm->vsl;
> +     dmode->vtotal = dmode->vsync_end + vm->vbp;
>  
>       dmode->clock = vm->pixelclock / 1000;
>  
> @@ -565,7 +565,7 @@ int of_get_drm_display_mode(struct device_node *np,
>       drm_display_mode_from_videomode(&vm, dmode);
>  
>       pr_debug("%s: got %dx%d display mode from %s\n",
> -             of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> +             of_node_full_name(np), vm.hact, vm.vact, np->name);
>       drm_mode_debug_printmodeline(dmode);
>  
>       return 0;
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index e5cc2fd..8103fc9 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1382,15 +1382,15 @@ int fb_videomode_from_videomode(const struct 
> videomode *vm,
>  {
>       unsigned int htotal, vtotal;
>  
> -     fbmode->xres = vm->hactive;
> -     fbmode->left_margin = vm->hback_porch;
> -     fbmode->right_margin = vm->hfront_porch;
> -     fbmode->hsync_len = vm->hsync_len;
> +     fbmode->xres = vm->hact;
> +     fbmode->left_margin = vm->hbp;
> +     fbmode->right_margin = vm->hfp;
> +     fbmode->hsync_len = vm->hsl;
>  
> -     fbmode->yres = vm->vactive;
> -     fbmode->upper_margin = vm->vback_porch;
> -     fbmode->lower_margin = vm->vfront_porch;
> -     fbmode->vsync_len = vm->vsync_len;
> +     fbmode->yres = vm->vact;
> +     fbmode->upper_margin = vm->vbp;
> +     fbmode->lower_margin = vm->vfp;
> +     fbmode->vsync_len = vm->vsl;
>  
>       /* prevent division by zero in KHZ2PICOS macro */
>       fbmode->pixclock = vm->pixelclock ?
> @@ -1408,10 +1408,8 @@ int fb_videomode_from_videomode(const struct videomode 
> *vm,
>               fbmode->vmode |= FB_VMODE_DOUBLE;
>       fbmode->flag = 0;
>  
> -     htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> -              vm->hsync_len;
> -     vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> -              vm->vsync_len;
> +     htotal = vm->hact + vm->hfp + vm->hbp + vm->hsl;
> +     vtotal = vm->vact + vm->vfp + vm->vbp + vm->vsl;
>       /* prevent division by zero */
>       if (htotal && vtotal) {
>               fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> @@ -1458,7 +1456,7 @@ int of_get_fb_videomode(struct device_node *np, struct 
> fb_videomode *fb,
>       fb_videomode_from_videomode(&vm, fb);
>  
>       pr_debug("%s: got %dx%d display mode from %s\n",
> -             of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> +             of_node_full_name(np), vm.hact, vm.vact, np->name);
>       dump_fb_videomode(fb);
>  
>       return 0;
> diff --git a/drivers/video/of_display_timing.c 
> b/drivers/video/of_display_timing.c
> index 56009bc..79660937 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -69,14 +69,14 @@ static struct display_timing 
> *of_get_display_timing(struct device_node *np)
>               return NULL;
>       }
>  
> -     ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
> -     ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
> -     ret |= parse_timing_property(np, "hactive", &dt->hactive);
> -     ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
> -     ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
> -     ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
> -     ret |= parse_timing_property(np, "vactive", &dt->vactive);
> -     ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
> +     ret |= parse_timing_property(np, "hback-porch", &dt->hbp);
> +     ret |= parse_timing_property(np, "hfront-porch", &dt->hfp);
> +     ret |= parse_timing_property(np, "hactive", &dt->hact);
> +     ret |= parse_timing_property(np, "hsync-len", &dt->hsl);
> +     ret |= parse_timing_property(np, "vback-porch", &dt->vbp);
> +     ret |= parse_timing_property(np, "vfront-porch", &dt->vfp);
> +     ret |= parse_timing_property(np, "vactive", &dt->vact);
> +     ret |= parse_timing_property(np, "vsync-len", &dt->vsl);
>       ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
>  
>       dt->flags = 0;
> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> index a3d95f2..c42689a 100644
> --- a/drivers/video/videomode.c
> +++ b/drivers/video/videomode.c
> @@ -21,15 +21,15 @@ int videomode_from_timing(const struct display_timings 
> *disp,
>               return -EINVAL;
>  
>       vm->pixelclock = dt->pixelclock.typ;
> -     vm->hactive = dt->hactive.typ;
> -     vm->hfront_porch = dt->hfront_porch.typ;
> -     vm->hback_porch = dt->hback_porch.typ;
> -     vm->hsync_len = dt->hsync_len.typ;
> +     vm->hact = dt->hact.typ;
> +     vm->hfp = dt->hfp.typ;
> +     vm->hbp = dt->hbp.typ;
> +     vm->hsl = dt->hsl.typ;
>  
> -     vm->vactive = dt->vactive.typ;
> -     vm->vfront_porch = dt->vfront_porch.typ;
> -     vm->vback_porch = dt->vback_porch.typ;
> -     vm->vsync_len = dt->vsync_len.typ;
> +     vm->vact = dt->vact.typ;
> +     vm->vfp = dt->vfp.typ;
> +     vm->vbp = dt->vbp.typ;
> +     vm->vsl = dt->vsl.typ;
>  
>       vm->flags = dt->flags;
>  
> diff --git a/include/video/display_timing.h b/include/video/display_timing.h
> index 5d0259b..db98ef9 100644
> --- a/include/video/display_timing.h
> +++ b/include/video/display_timing.h
> @@ -59,15 +59,15 @@ struct timing_entry {
>  struct display_timing {
>       struct timing_entry pixelclock;
>  
> -     struct timing_entry hactive;            /* hor. active video */
> -     struct timing_entry hfront_porch;       /* hor. front porch */
> -     struct timing_entry hback_porch;        /* hor. back porch */
> -     struct timing_entry hsync_len;          /* hor. sync len */
> +     struct timing_entry hact;               /* hor. active video */
> +     struct timing_entry hfp;                /* hor. front porch */
> +     struct timing_entry hbp;                /* hor. back porch */
> +     struct timing_entry hsl;                /* hor. sync len */
>  
> -     struct timing_entry vactive;            /* ver. active video */
> -     struct timing_entry vfront_porch;       /* ver. front porch */
> -     struct timing_entry vback_porch;        /* ver. back porch */
> -     struct timing_entry vsync_len;          /* ver. sync len */
> +     struct timing_entry vact;               /* ver. active video */
> +     struct timing_entry vfp;                /* ver. front porch */
> +     struct timing_entry vbp;                /* ver. back porch */
> +     struct timing_entry vsl;                /* ver. sync len */
>  
>       enum display_flags flags;               /* display flags */
>  };
> diff --git a/include/video/videomode.h b/include/video/videomode.h
> index 8b12e60..b601c0c 100644
> --- a/include/video/videomode.h
> +++ b/include/video/videomode.h
> @@ -19,15 +19,15 @@
>  struct videomode {
>       unsigned long pixelclock;       /* pixelclock in Hz */
>  
> -     u32 hactive;
> -     u32 hfront_porch;
> -     u32 hback_porch;
> -     u32 hsync_len;
> -
> -     u32 vactive;
> -     u32 vfront_porch;
> -     u32 vback_porch;
> -     u32 vsync_len;
> +     u32 hact;
> +     u32 hfp;
> +     u32 hbp;
> +     u32 hsl;
> +
> +     u32 vact;
> +     u32 vfp;
> +     u32 vbp;
> +     u32 vsl;
>  
>       enum display_flags flags; /* display flags */
>  };
> 

Hi,

I really don't like this. It may be shorter, but I think it makes it really
hard to read. And the direct mapping of DT<->C is lost.

Regards,
Steffen


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Reply via email to