Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: [...] > diff --git a/include/linux/of_display_timings.h > b/include/linux/of_display_timings.h [...] > +#ifndef __LINUX_OF_DISPLAY_TIMINGS_H > +#define __LINUX_OF_DISPLAY_TIMINGS_H > + > +#include > + > +#define OF_USE_NATIVE_MODE -1 > + > +struct display_timings *of_get_display_timings(struct device_node *np); > +int of_display_timings_exists(struct device_node *np); This either needs to include linux/of.h or a forward declaration of struct device_node. Otherwise this will fail to compile if the file where this is included from doesn't pull linux/of.h in explicitly. > diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h [...] > +#ifndef __LINUX_OF_VIDEOMODE_H > +#define __LINUX_OF_VIDEOMODE_H > + > +#include > + > +int of_get_videomode(struct device_node *np, struct videomode *vm, int > index); Same here. Thierry pgpDu4puXcSWO.pgp Description: PGP signature
Re: [PATCH v8 2/6] video: add of helper for videomode
On Tue, Nov 13, 2012 at 08:13:31AM -1000, Mitch Bradley wrote: > On 11/13/2012 7:51 AM, Thierry Reding wrote: > > On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote: > >> On 11/13/2012 04:08 AM, Thierry Reding wrote: > >>> On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: > This adds support for reading display timings from DT or/and > convert one of those timings to a videomode. The > of_display_timing implementation supports multiple children where > each property can have up to 3 values. All children are read into > an array, that can be queried. of_get_videomode converts exactly > one of that timings to a struct videomode. > >> > diff --git > a/Documentation/devicetree/bindings/video/display-timings.txt > b/Documentation/devicetree/bindings/video/display-timings.txt > >> > + - clock-frequency: displayclock in Hz > >>> > >>> "display clock"? > >> > >> I /think/ I had suggested naming this clock-frequency before so that > >> the property name would be more standardized; other bindings use that > >> same name. But I'm not too attached to the name I guess. > > > > That's not what I meant. I think "displayclock" should be two words in > > the description of the property. The property name is fine. > > Given that modern display engines often have numerous clocks, perhaps it > would be better to use a more specific name, like for example "pixel-clock". This binding is only about defining display modes. Are any of the clocks that you're referring to relevant to the actual display modes? Thierry pgpII2bCgfQhH.pgp Description: PGP signature
Re: [PATCH v8 2/6] video: add of helper for videomode
On 11/13/2012 7:51 AM, Thierry Reding wrote: > On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote: >> On 11/13/2012 04:08 AM, Thierry Reding wrote: >>> On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: This adds support for reading display timings from DT or/and convert one of those timings to a videomode. The of_display_timing implementation supports multiple children where each property can have up to 3 values. All children are read into an array, that can be queried. of_get_videomode converts exactly one of that timings to a struct videomode. >> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt >> + - clock-frequency: displayclock in Hz >>> >>> "display clock"? >> >> I /think/ I had suggested naming this clock-frequency before so that >> the property name would be more standardized; other bindings use that >> same name. But I'm not too attached to the name I guess. > > That's not what I meant. I think "displayclock" should be two words in > the description of the property. The property name is fine. Given that modern display engines often have numerous clocks, perhaps it would be better to use a more specific name, like for example "pixel-clock". > > Thierry > > > > ___ > devicetree-discuss mailing list > devicetree-disc...@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote: > On 11/13/2012 04:08 AM, Thierry Reding wrote: > > On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: > >> This adds support for reading display timings from DT or/and > >> convert one of those timings to a videomode. The > >> of_display_timing implementation supports multiple children where > >> each property can have up to 3 values. All children are read into > >> an array, that can be queried. of_get_videomode converts exactly > >> one of that timings to a struct videomode. > > >> diff --git > >> a/Documentation/devicetree/bindings/video/display-timings.txt > >> b/Documentation/devicetree/bindings/video/display-timings.txt > > >> + - clock-frequency: displayclock in Hz > > > > "display clock"? > > I /think/ I had suggested naming this clock-frequency before so that > the property name would be more standardized; other bindings use that > same name. But I'm not too attached to the name I guess. That's not what I meant. I think "displayclock" should be two words in the description of the property. The property name is fine. Thierry pgpRQ9Pi5xJg9.pgp Description: PGP signature
Re: [PATCH v8 2/6] video: add of helper for videomode
On 11/13/2012 04:08 AM, Thierry Reding wrote: > On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: >> This adds support for reading display timings from DT or/and >> convert one of those timings to a videomode. The >> of_display_timing implementation supports multiple children where >> each property can have up to 3 values. All children are read into >> an array, that can be queried. of_get_videomode converts exactly >> one of that timings to a struct videomode. >> diff --git >> a/Documentation/devicetree/bindings/video/display-timings.txt >> b/Documentation/devicetree/bindings/video/display-timings.txt >> + - clock-frequency: displayclock in Hz > > "display clock"? I /think/ I had suggested naming this clock-frequency before so that the property name would be more standardized; other bindings use that same name. But I'm not too attached to the name I guess. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 01:40:12PM -0700, Stephen Warren wrote: > On 11/12/2012 08:37 AM, Steffen Trumtrar wrote: > > This adds support for reading display timings from DT or/and convert one of > > those > > timings to a videomode. > > The of_display_timing implementation supports multiple children where each > > property can have up to 3 values. All children are read into an array, that > > can be queried. > > of_get_videomode converts exactly one of that timings to a struct videomode. > > > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > > b/Documentation/devicetree/bindings/video/display-timings.txt > > The device tree bindings look reasonable to me, so, > > Acked-by: Stephen Warren > > Thanks, 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- | -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: > This adds support for reading display timings from DT or/and convert one of > those > timings to a videomode. > The of_display_timing implementation supports multiple children where each > property can have up to 3 values. All children are read into an array, that > can be queried. > of_get_videomode converts exactly one of that timings to a struct videomode. > > Signed-off-by: Steffen Trumtrar > Signed-off-by: Philipp Zabel > --- > .../devicetree/bindings/video/display-timings.txt | 107 +++ > drivers/video/Kconfig | 13 ++ > drivers/video/Makefile |2 + > drivers/video/of_display_timing.c | 186 > > drivers/video/of_videomode.c | 47 + > include/linux/of_display_timings.h | 19 ++ > include/linux/of_videomode.h | 15 ++ > 7 files changed, 389 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/video/display-timings.txt > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 include/linux/of_display_timings.h > create mode 100644 include/linux/of_videomode.h > > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > b/Documentation/devicetree/bindings/video/display-timings.txt > new file mode 100644 > index 000..e22e2fd > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/display-timings.txt > @@ -0,0 +1,107 @@ > +display-timings bindings > + > + > +display-timings-node > + Maybe leave away the last -, so that it reads "display-timings node"? I think that makes it more obvious that the node is supposed to be called "display-timings". > + > +required properties: > + - none > + > +optional properties: > + - native-mode: the native mode for the display, in case multiple modes are > + provided. When omitted, assume the first node is the native. > + > +timings-subnode > +--- Same here: "timing subnodes"? > + > +required properties: > + - hactive, vactive: Display resolution > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters "display"? > + in pixels > + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters > in > + lines > + - clock-frequency: displayclock in Hz "display clock"? > + > +optional properties: > + - hsync-active : Hsync pulse is active low/high/ignored > + - vsync-active : Vsync pulse is active low/high/ignored > + - de-active : Data-Enable pulse is active low/high/ignored > + - pixelclk-inverted : pixelclock is inverted/non-inverted/ignored > + - interlaced (bool) > + - doublescan (bool) > + > +All the optional properties that are not bool follow the following logic: > +<1> : high active > +<0> : low active > +omitted : not used on hardware Nitpick: You use space before : in the optional properties, but not in the required properties above. > + > +There are different ways of describing the capabilities of a display. The > devicetree > +representation corresponds to the one commonly found in datasheets for > displays. > +If a display supports multiple signal timings, the native-mode can be > specified. > + > +The parameters are defined as > + > +struct display_timing > += struct display_timing has no meaning in device tree documentation. Maybe this line can just go away? > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index 2a23b18..a324457 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -39,6 +39,19 @@ config DISPLAY_TIMING > config VIDEOMODE > bool > > +config OF_DISPLAY_TIMING OF_DISPLAY_TIMINGS? > diff --git a/drivers/video/of_display_timing.c > b/drivers/video/of_display_timing.c of_display_timings.c? > +/** > + * of_get_display_timings - parse all display_timing entries from a > device_node > + * @np: device_node with the subnodes > + **/ > +struct display_timings *of_get_display_timings(struct device_node *np) [...] > + for_each_child_of_node(timings_np, entry) { > + struct display_timing *dt; > + > + dt = of_get_display_timing(entry); > + if (!dt) { > + /* to not encourage wrong devicetrees, fail in case of > an error */ > + pr_err("%s: error in timing %d\n", __func__, > disp->num_timings+1); > + return NULL; > + } In case of a parsing error, of_get_display_timing() already shows an error message, so I don't think we need another one here. > +/** > + * of_display_timings_exists - check if a display-timings node is provided > + * @np: device_node with the timing > + **/ > +int of_display_timings_exists(struct device_node *np) > +{ > + struct device_node *timings_np; > +
Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 11:00:37PM +0400, Alexey Klimov wrote: > Hello Steffen, > > On Mon, Nov 12, 2012 at 7:37 PM, Steffen Trumtrar > wrote: > > This adds support for reading display timings from DT or/and convert one of > > those > > timings to a videomode. > > The of_display_timing implementation supports multiple children where each > > property can have up to 3 values. All children are read into an array, that > > can be queried. > > of_get_videomode converts exactly one of that timings to a struct videomode. > > > > Signed-off-by: Steffen Trumtrar > > Signed-off-by: Philipp Zabel > > --- > > .../devicetree/bindings/video/display-timings.txt | 107 +++ > > drivers/video/Kconfig | 13 ++ > > drivers/video/Makefile |2 + > > drivers/video/of_display_timing.c | 186 > > > > drivers/video/of_videomode.c | 47 + > > include/linux/of_display_timings.h | 19 ++ > > include/linux/of_videomode.h | 15 ++ > > 7 files changed, 389 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/video/display-timings.txt > > create mode 100644 drivers/video/of_display_timing.c > > create mode 100644 drivers/video/of_videomode.c > > create mode 100644 include/linux/of_display_timings.h > > create mode 100644 include/linux/of_videomode.h > > > > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > > b/Documentation/devicetree/bindings/video/display-timings.txt > > new file mode 100644 > > index 000..e22e2fd > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/video/display-timings.txt > > @@ -0,0 +1,107 @@ > > +display-timings bindings > > + > > + > > +display-timings-node > > + > > + > > +required properties: > > + - none > > + > > +optional properties: > > + - native-mode: the native mode for the display, in case multiple modes are > > + provided. When omitted, assume the first node is the native. > > + > > +timings-subnode > > +--- > > + > > +required properties: > > + - hactive, vactive: Display resolution > > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing > > parameters > > + in pixels > > + vfront-porch, vback-porch, vsync-len: Vertical display timing > > parameters in > > + lines > > + - clock-frequency: displayclock in Hz > > + > > +optional properties: > > + - hsync-active : Hsync pulse is active low/high/ignored > > + - vsync-active : Vsync pulse is active low/high/ignored > > + - de-active : Data-Enable pulse is active low/high/ignored > > + - pixelclk-inverted : pixelclock is inverted/non-inverted/ignored > > + - interlaced (bool) > > + - doublescan (bool) > > + > > +All the optional properties that are not bool follow the following logic: > > +<1> : high active > > +<0> : low active > > +omitted : not used on hardware > > + > > +There are different ways of describing the capabilities of a display. The > > devicetree > > +representation corresponds to the one commonly found in datasheets for > > displays. > > +If a display supports multiple signal timings, the native-mode can be > > specified. > > + > > +The parameters are defined as > > + > > +struct display_timing > > += > > + > > + > > +--+-+--+---+ > > + | |↑| | > >| > > + | ||vback_porch | | > >| > > + | |↓| | > >| > > + > > +--###--+---+ > > + | #↑# | > >| > > + | #|# | > >| > > + | hback #|# hfront | > > hsync | > > + | porch #| hactive # porch | > > len | > > + > > |<>#<---+--->#<>|<->| > > + | #|# | > >| > > + | #|vactive # | > >| > > + | #|# | > >| > > + | #↓# | > >| > > + > > +--###--+---+ > > + | |↑| | > >| > > + | ||vfront_porch| | > >| > > + | |↓
Re: [PATCH v8 2/6] video: add of helper for videomode
Hi! On Mon, Nov 12, 2012 at 07:58:40PM +0100, Sascha Hauer wrote: > Hi Steffen, > > You lose memory in several places: > > On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: > > +static struct display_timing *of_get_display_timing(struct device_node *np) > > +{ > > + struct display_timing *dt; > > + int ret = 0; > > + > > + dt = kzalloc(sizeof(*dt), GFP_KERNEL); > > + if (!dt) { > > + pr_err("%s: could not allocate display_timing struct\n", > > __func__); > > + return NULL; > > + } > > + > > + ret |= parse_property(np, "hback-porch", &dt->hback_porch); > > + ret |= parse_property(np, "hfront-porch", &dt->hfront_porch); > > + ret |= parse_property(np, "hactive", &dt->hactive); > > + ret |= parse_property(np, "hsync-len", &dt->hsync_len); > > + ret |= parse_property(np, "vback-porch", &dt->vback_porch); > > + ret |= parse_property(np, "vfront-porch", &dt->vfront_porch); > > + ret |= parse_property(np, "vactive", &dt->vactive); > > + ret |= parse_property(np, "vsync-len", &dt->vsync_len); > > + ret |= parse_property(np, "clock-frequency", &dt->pixelclock); > > + > > + of_property_read_u32(np, "vsync-active", &dt->vsync_pol_active); > > + of_property_read_u32(np, "hsync-active", &dt->hsync_pol_active); > > + of_property_read_u32(np, "de-active", &dt->de_pol_active); > > + of_property_read_u32(np, "pixelclk-inverted", &dt->pixelclk_pol); > > + dt->interlaced = of_property_read_bool(np, "interlaced"); > > + dt->doublescan = of_property_read_bool(np, "doublescan"); > > + > > + if (ret) { > > + pr_err("%s: error reading timing properties\n", __func__); > > + return NULL; > > Here > > > + } > > + > > + return dt; > > +} > > + > > +/** > > + * of_get_display_timings - parse all display_timing entries from a > > device_node > > + * @np: device_node with the subnodes > > + **/ > > +struct display_timings *of_get_display_timings(struct device_node *np) > > +{ > > + struct device_node *timings_np; > > + struct device_node *entry; > > + struct device_node *native_mode; > > + struct display_timings *disp; > > + > > + if (!np) { > > + pr_err("%s: no devicenode given\n", __func__); > > + return NULL; > > + } > > + > > + timings_np = of_find_node_by_name(np, "display-timings"); > > + if (!timings_np) { > > + pr_err("%s: could not find display-timings node\n", __func__); > > + return NULL; > > + } > > + > > + disp = kzalloc(sizeof(*disp), GFP_KERNEL); > > + if (!disp) > > + return -ENOMEM; > > + > > + entry = of_parse_phandle(timings_np, "native-mode", 0); > > + /* assume first child as native mode if none provided */ > > + if (!entry) > > + entry = of_get_next_child(np, NULL); > > + if (!entry) { > > + pr_err("%s: no timing specifications given\n", __func__); > > + return NULL; > > Here > > > + } > > + > > + pr_info("%s: using %s as default timing\n", __func__, entry->name); > > + > > + native_mode = entry; > > + > > + disp->num_timings = of_get_child_count(timings_np); > > + disp->timings = kzalloc(sizeof(struct display_timing > > *)*disp->num_timings, > > + GFP_KERNEL); > > + if (!disp->timings) > > + return -ENOMEM; > > Here > > > + > > + disp->num_timings = 0; > > + disp->native_mode = 0; > > + > > + for_each_child_of_node(timings_np, entry) { > > + struct display_timing *dt; > > + > > + dt = of_get_display_timing(entry); > > + if (!dt) { > > + /* to not encourage wrong devicetrees, fail in case of > > an error */ > > + pr_err("%s: error in timing %d\n", __func__, > > disp->num_timings+1); > > + return NULL; > > Here > > > + } > > + > > + if (native_mode == entry) > > + disp->native_mode = disp->num_timings; > > + > > + disp->timings[disp->num_timings] = dt; > > + disp->num_timings++; > > + } > > + of_node_put(timings_np); > > + > > + if (disp->num_timings > 0) > > + pr_info("%s: got %d timings. Using timing #%d as default\n", > > __func__, > > + disp->num_timings , disp->native_mode + 1); > > + else { > > + pr_err("%s: no valid timings specified\n", __func__); > > + return NULL; > > and here > Well,... you are right. :-( 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- | -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
On 11/12/2012 08:37 AM, Steffen Trumtrar wrote: > This adds support for reading display timings from DT or/and convert one of > those > timings to a videomode. > The of_display_timing implementation supports multiple children where each > property can have up to 3 values. All children are read into an array, that > can be queried. > of_get_videomode converts exactly one of that timings to a struct videomode. > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > b/Documentation/devicetree/bindings/video/display-timings.txt The device tree bindings look reasonable to me, so, Acked-by: Stephen Warren -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
Hello Steffen, On Mon, Nov 12, 2012 at 7:37 PM, Steffen Trumtrar wrote: > This adds support for reading display timings from DT or/and convert one of > those > timings to a videomode. > The of_display_timing implementation supports multiple children where each > property can have up to 3 values. All children are read into an array, that > can be queried. > of_get_videomode converts exactly one of that timings to a struct videomode. > > Signed-off-by: Steffen Trumtrar > Signed-off-by: Philipp Zabel > --- > .../devicetree/bindings/video/display-timings.txt | 107 +++ > drivers/video/Kconfig | 13 ++ > drivers/video/Makefile |2 + > drivers/video/of_display_timing.c | 186 > > drivers/video/of_videomode.c | 47 + > include/linux/of_display_timings.h | 19 ++ > include/linux/of_videomode.h | 15 ++ > 7 files changed, 389 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/video/display-timings.txt > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 include/linux/of_display_timings.h > create mode 100644 include/linux/of_videomode.h > > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > b/Documentation/devicetree/bindings/video/display-timings.txt > new file mode 100644 > index 000..e22e2fd > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/display-timings.txt > @@ -0,0 +1,107 @@ > +display-timings bindings > + > + > +display-timings-node > + > + > +required properties: > + - none > + > +optional properties: > + - native-mode: the native mode for the display, in case multiple modes are > + provided. When omitted, assume the first node is the native. > + > +timings-subnode > +--- > + > +required properties: > + - hactive, vactive: Display resolution > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters > + in pixels > + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters > in > + lines > + - clock-frequency: displayclock in Hz > + > +optional properties: > + - hsync-active : Hsync pulse is active low/high/ignored > + - vsync-active : Vsync pulse is active low/high/ignored > + - de-active : Data-Enable pulse is active low/high/ignored > + - pixelclk-inverted : pixelclock is inverted/non-inverted/ignored > + - interlaced (bool) > + - doublescan (bool) > + > +All the optional properties that are not bool follow the following logic: > +<1> : high active > +<0> : low active > +omitted : not used on hardware > + > +There are different ways of describing the capabilities of a display. The > devicetree > +representation corresponds to the one commonly found in datasheets for > displays. > +If a display supports multiple signal timings, the native-mode can be > specified. > + > +The parameters are defined as > + > +struct display_timing > += > + > + > +--+-+--+---+ > + | |↑| | > | > + | ||vback_porch | | > | > + | |↓| | > | > + > +--###--+---+ > + | #↑# | > | > + | #|# | > | > + | hback #|# hfront | > hsync | > + | porch #| hactive # porch | len > | > + > |<>#<---+--->#<>|<->| > + | #|# | > | > + | #|vactive # | > | > + | #|# | > | > + | #↓# | > | > + > +--###--+---+ > + | |↑| | > | > + | ||vfront_porch| | > | > + | |↓| | > | > + > +--+-+--+---+ > + | |↑| | > | > + | ||vsync_len | | > | > + | |
Re: [PATCH v8 2/6] video: add of helper for videomode
Hi Steffen, You lose memory in several places: On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote: > +static struct display_timing *of_get_display_timing(struct device_node *np) > +{ > + struct display_timing *dt; > + int ret = 0; > + > + dt = kzalloc(sizeof(*dt), GFP_KERNEL); > + if (!dt) { > + pr_err("%s: could not allocate display_timing struct\n", > __func__); > + return NULL; > + } > + > + ret |= parse_property(np, "hback-porch", &dt->hback_porch); > + ret |= parse_property(np, "hfront-porch", &dt->hfront_porch); > + ret |= parse_property(np, "hactive", &dt->hactive); > + ret |= parse_property(np, "hsync-len", &dt->hsync_len); > + ret |= parse_property(np, "vback-porch", &dt->vback_porch); > + ret |= parse_property(np, "vfront-porch", &dt->vfront_porch); > + ret |= parse_property(np, "vactive", &dt->vactive); > + ret |= parse_property(np, "vsync-len", &dt->vsync_len); > + ret |= parse_property(np, "clock-frequency", &dt->pixelclock); > + > + of_property_read_u32(np, "vsync-active", &dt->vsync_pol_active); > + of_property_read_u32(np, "hsync-active", &dt->hsync_pol_active); > + of_property_read_u32(np, "de-active", &dt->de_pol_active); > + of_property_read_u32(np, "pixelclk-inverted", &dt->pixelclk_pol); > + dt->interlaced = of_property_read_bool(np, "interlaced"); > + dt->doublescan = of_property_read_bool(np, "doublescan"); > + > + if (ret) { > + pr_err("%s: error reading timing properties\n", __func__); > + return NULL; Here > + } > + > + return dt; > +} > + > +/** > + * of_get_display_timings - parse all display_timing entries from a > device_node > + * @np: device_node with the subnodes > + **/ > +struct display_timings *of_get_display_timings(struct device_node *np) > +{ > + struct device_node *timings_np; > + struct device_node *entry; > + struct device_node *native_mode; > + struct display_timings *disp; > + > + if (!np) { > + pr_err("%s: no devicenode given\n", __func__); > + return NULL; > + } > + > + timings_np = of_find_node_by_name(np, "display-timings"); > + if (!timings_np) { > + pr_err("%s: could not find display-timings node\n", __func__); > + return NULL; > + } > + > + disp = kzalloc(sizeof(*disp), GFP_KERNEL); > + if (!disp) > + return -ENOMEM; > + > + entry = of_parse_phandle(timings_np, "native-mode", 0); > + /* assume first child as native mode if none provided */ > + if (!entry) > + entry = of_get_next_child(np, NULL); > + if (!entry) { > + pr_err("%s: no timing specifications given\n", __func__); > + return NULL; Here > + } > + > + pr_info("%s: using %s as default timing\n", __func__, entry->name); > + > + native_mode = entry; > + > + disp->num_timings = of_get_child_count(timings_np); > + disp->timings = kzalloc(sizeof(struct display_timing > *)*disp->num_timings, > + GFP_KERNEL); > + if (!disp->timings) > + return -ENOMEM; Here > + > + disp->num_timings = 0; > + disp->native_mode = 0; > + > + for_each_child_of_node(timings_np, entry) { > + struct display_timing *dt; > + > + dt = of_get_display_timing(entry); > + if (!dt) { > + /* to not encourage wrong devicetrees, fail in case of > an error */ > + pr_err("%s: error in timing %d\n", __func__, > disp->num_timings+1); > + return NULL; Here > + } > + > + if (native_mode == entry) > + disp->native_mode = disp->num_timings; > + > + disp->timings[disp->num_timings] = dt; > + disp->num_timings++; > + } > + of_node_put(timings_np); > + > + if (disp->num_timings > 0) > + pr_info("%s: got %d timings. Using timing #%d as default\n", > __func__, > + disp->num_timings , disp->native_mode + 1); > + else { > + pr_err("%s: no valid timings specified\n", __func__); > + return NULL; and here Sascha -- 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- | -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html