diff mbox

[5/5] videomode: rename fields

Message ID 1363083578-17062-5-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen March 12, 2013, 10:19 a.m. UTC
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@ti.com>
Cc: Steffen Trumtrar <s.trumtrar@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(-)

Comments

Laurent Pinchart March 12, 2013, 1:37 p.m. UTC | #1
Hi Tomi,

Thanks for the patch.

On Tuesday 12 March 2013 12:19:38 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@ti.com>
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---

I have no strong opinion on this, but I find the existing names easier to 
read. I might be biased by having read them often though.
Tomi Valkeinen March 12, 2013, 1:40 p.m. UTC | #2
Hi,

On 2013-03-12 15:37, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thanks for the patch.
> 
> On Tuesday 12 March 2013 12:19:38 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@ti.com>
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> ---
> 
> I have no strong opinion on this, but I find the existing names easier to 
> read. I might be biased by having read them often though.

Yes, the last patch was a bit of a "teaser" =). I found myself typoing
them a lot, using helper local variables to shorten the code lines, and
as I mention in the description, I find them a bit of a mishmash. So,
while they're not used in any drivers yet, I thought it'd be worth a
shot to change them.

 Tomi
Steffen Trumtrar March 12, 2013, 1:53 p.m. UTC | #3
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@ti.com>
> Cc: Steffen Trumtrar <s.trumtrar@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
Daniel Vetter March 18, 2013, 8 a.m. UTC | #4
On Tue, Mar 12, 2013 at 03:40:14PM +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 2013-03-12 15:37, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > Thanks for the patch.
> > 
> > On Tuesday 12 March 2013 12:19:38 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@ti.com>
> >> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> >> ---
> > 
> > I have no strong opinion on this, but I find the existing names easier to 
> > read. I might be biased by having read them often though.
> 
> Yes, the last patch was a bit of a "teaser" =). I found myself typoing
> them a lot, using helper local variables to shorten the code lines, and
> as I mention in the description, I find them a bit of a mishmash. So,
> while they're not used in any drivers yet, I thought it'd be worth a
> shot to change them.

Imo the new names look quite a bit more ugly and less readable, for very
few saved chars. And at least for i915.ko development it's been a long
time since I've last had to type them ... Maybe the real problem is that
we have a few too many video mode structures and can't properly share
them?
-Daniel
Tomi Valkeinen March 18, 2013, 12:28 p.m. UTC | #5
On 2013-03-18 10:00, Daniel Vetter wrote:
> On Tue, Mar 12, 2013 at 03:40:14PM +0200, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 2013-03-12 15:37, Laurent Pinchart wrote:
>>> Hi Tomi,
>>>
>>> Thanks for the patch.
>>>
>>> On Tuesday 12 March 2013 12:19:38 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@ti.com>
>>>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>>>> ---
>>>
>>> I have no strong opinion on this, but I find the existing names easier to 
>>> read. I might be biased by having read them often though.
>>
>> Yes, the last patch was a bit of a "teaser" =). I found myself typoing
>> them a lot, using helper local variables to shorten the code lines, and
>> as I mention in the description, I find them a bit of a mishmash. So,
>> while they're not used in any drivers yet, I thought it'd be worth a
>> shot to change them.
> 
> Imo the new names look quite a bit more ugly and less readable, for very
> few saved chars. And at least for i915.ko development it's been a long
> time since I've last had to type them ... Maybe the real problem is that
> we have a few too many video mode structures and can't properly share
> them?

I guess it's a matter of taste. But I've received three "I don't like
the new names" comments, and no positive comments, so I'm dropping the
last patch. The first four should be fine, though.

And yes, we should definitely try to use only this new videomode struct
in the future everywhere in the kernel. It sure would make me remember
the names better =).

 Tomi
diff mbox

Patch

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 */
 };