On Fri, Sep 14, 2012 at 11:24 AM, Steffen Trumtrar s.trumtrar@pengutronix.de wrote:
+/* FIXME */ +static u32 of_video_get_value(struct mode_property *prop) +{
return (prop->min >= prop->typ) ? prop->min : prop->typ;+}
+/* read property into new mode_property */ +static int of_video_parse_property(struct device_node *np, char *name,
struct mode_property *result)+{
struct property *prop;int length;int cells;int ret;prop = of_find_property(np, name, &length);if (!prop)return -EINVAL;cells = length / sizeof(u32);memset(result, 0, sizeof(*result));ret = of_property_read_u32_array(np, name, &result->min, cells);of_video_get_value(result);
What's the point of calling of_video_get_value() here? It doesn't do anything.
return ret;+}
+int videomode_to_display_mode(struct display *disp, struct drm_display_mode *dmode, int index) +{
struct videomode *vm;memset(dmode, 0, sizeof(*dmode));
Indentation problem.
+int of_get_video_mode(struct device_node *np, struct display *disp) +{
struct device_node *display_np;struct device_node *mode_np;struct device_node *modes_np;char *default_mode;int ret = 0;memset(disp, 0, sizeof(*disp));disp->modes = kmalloc(sizeof(struct videomode*), GFP_KERNEL);if (!np)return -EINVAL;display_np = of_parse_phandle(np, "display", 0);if (!display_np)return -EINVAL;of_property_read_u32(display_np, "width-mm", &disp->width_mm);of_property_read_u32(display_np, "height-mm", &disp->height_mm);mode_np = of_parse_phandle(np, "default-mode", 0);if (!mode_np)mode_np = of_find_node_by_name(np, "mode");if (!mode_np)return -EINVAL;default_mode = (char *)mode_np->full_name;modes_np = of_find_node_by_name(np, "modes");for_each_child_of_node(modes_np, mode_np) {struct videomode *vm;
Blank line after variable declarations, please.
vm = kmalloc(sizeof(struct videomode), GFP_KERNEL);disp->modes[disp->num_modes] = kmalloc(sizeof(struct videomode*), GFP_KERNEL);
Are you sure this is right?
ret |= of_video_parse_property(mode_np, "hback-porch", &vm->hback_porch);ret |= of_video_parse_property(mode_np, "hfront-porch", &vm->hfront_porch);ret |= of_video_parse_property(mode_np, "hactive", &vm->hactive);ret |= of_video_parse_property(mode_np, "hsync-len", &vm->hsync_len);ret |= of_video_parse_property(mode_np, "vback-porch", &vm->vback_porch);ret |= of_video_parse_property(mode_np, "vfront-porch", &vm->vfront_porch);ret |= of_video_parse_property(mode_np, "vactive", &vm->vactive);ret |= of_video_parse_property(mode_np, "vsync-len", &vm->vsync_len);ret |= of_video_parse_property(mode_np, "clock", &vm->clock);if (ret)return -EINVAL;vm->hah = of_property_read_bool(mode_np, "hsync-active-high");vm->vah = of_property_read_bool(mode_np, "vsync-active-high");vm->interlaced = of_property_read_bool(mode_np, "interlaced");vm->doublescan = of_property_read_bool(mode_np, "doublescan");if (strcmp(default_mode,mode_np->full_name) == 0) {printk("%s: default_node = %d\n", __func__, disp->num_modes);
Please use a KERN_ macro here, or a pr_xxx function. Even better would be to use a dev_xxx function.
In general, I'd like to see more error reporting of bad device tree properties, to help debugging.
disp->default_mode = disp->num_modes;}disp->modes[disp->num_modes] = vm;
Isn't this a memory leak?
disp->num_modes++;}of_node_put(display_np);return 0;+} +EXPORT_SYMBOL_GPL(of_get_video_mode);