On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote: [...]
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index d08d799..2a23b18 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL This framework adds support for low-level control of the video output switch.
+config DISPLAY_TIMING
DISPLAY_TIMINGS?
#video output switch sysfs driver obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o +obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
display_timings.o?
+obj-$(CONFIG_VIDEOMODE) += videomode.o diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
display_timings.c?
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
unsigned int index)
I find the indexing API a bit confusing. But that's perhaps just a matter of personal preference.
Also the ordering of arguments seems a little off. I find it more natural to have the destination pointer in the first argument, similar to the memcpy() function, so this would be:
int videomode_from_timing(struct videomode *vm, struct display_timings *disp, unsigned int index);
Actually, when reading videomode_from_timing() I'd expect the argument list to be:
int videomode_from_timing(struct videomode *vm, struct display_timing *timing);
Am I the only one confused by this?
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
display_timings.h?
+/* placeholder function until ranges are really needed
The above line has trailing whitespace. Also the block comment should have the opening /* on a separate line.
- the index parameter should then be used to select one of [min typ max]
If index is supposed to select min, typ or max, then maybe an enum would be a better candidate? Or alternatively provide separate accessors, like display_timing_get_{minimum,typical,maximum}().
- */
+static inline u32 display_timing_get_value(struct timing_entry *te,
unsigned int index)+{
- return te->typ;
+}
+static inline struct display_timing *display_timings_get(struct display_timings *disp,
unsigned int index)+{
- if (disp->num_timings > index)
return disp->timings[index];- else
return NULL;+}
+void timings_release(struct display_timings *disp);
This function no longer exists.
Thierry