* [PATCH v3 1/2] ARM: dts: da850-lcdk: add the dumb-vga-dac node
From: Bartosz Golaszewski @ 2016-11-29 11:57 UTC (permalink / raw)
To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski,
Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart
In-Reply-To: <1480420624-23544-1-git-send-email-bgolaszewski@baylibre.com>
Add the dumb-vga-dac node to the board DT together with corresponding
ports and vga connector. This allows to retrieve the edid info from
the display automatically.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/da850.dtsi | 17 ++++++++++++
2 files changed, 75 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 711b9ad..d864f11 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -50,6 +50,53 @@
system-clock-frequency = <24576000>;
};
};
+
+ vga_bridge {
+ compatible = "dumb-vga-dac";
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd_pins>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_bridge_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&display_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_bridge_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ vga {
+ compatible = "vga-connector";
+
+ ddc-i2c-bus = <&i2c0>;
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_bridge_out>;
+ };
+ };
+ };
};
&pmx_core {
@@ -235,3 +282,14 @@
&memctrl {
status = "okay";
};
+
+&display {
+ status = "okay";
+};
+
+&display_out {
+ display_out_vga: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_bridge_in>;
+ };
+};
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 4070619..5f4ba2e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -454,6 +454,23 @@
reg = <0x213000 0x1000>;
interrupts = <52>;
status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ display_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ display_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+ };
};
};
aemif: aemif@68000000 {
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH v3 0/2] ARM: dts: da850: tilcdc related DT changes
From: Bartosz Golaszewski @ 2016-11-29 11:57 UTC (permalink / raw)
To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart,
Bartosz Golaszewski
his series contains the last DT changes required for LCDC support
on da850-lcdk. The first one adds the dumb-vga-dac nodes, the second
limits the maximum pixel clock rate.
v1 -> v2:
- drop patch 3/3 (already merged)
- use max-pixelclock instead of max-bandwidth for display mode limiting
v2 -> v3:
- make the commit message in patch [2/2] more detailed
- move the max-pixelclock property to da850.dtsi as the limit
affects all da850-based boards
Bartosz Golaszewski (2):
ARM: dts: da850-lcdk: add the dumb-vga-dac node
ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/da850.dtsi | 18 +++++++++++++
2 files changed, 76 insertions(+)
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10 2/4] dtc: Document the dynamic plugin internals
From: Pantelis Antoniou @ 2016-11-29 11:21 UTC (permalink / raw)
To: Frank Rowand
Cc: David Gibson, Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <583CDB95.5000902-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Frank,
> On Nov 29, 2016, at 03:36 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> On 11/25/16 04:32, Pantelis Antoniou wrote:
>> Provides the document explaining the internal mechanics of
>> plugins and options.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> ---
>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 318 insertions(+)
>> create mode 100644 Documentation/dt-object-internal.txt
>>
>> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
>> new file mode 100644
>> index 0000000..d5b841e
>> --- /dev/null
>> +++ b/Documentation/dt-object-internal.txt
>> @@ -0,0 +1,318 @@
>> +Device Tree Dynamic Object format internals
>> +-------------------------------------------
>> +
>> +The Device Tree for most platforms is a static representation of
>> +the hardware capabilities. This is insufficient for many platforms
>> +that need to dynamically insert device tree fragments to the
>> +running kernel's live tree.
>> +
>> +This document explains the the device tree object format and the
>> +modifications made to the device tree compiler, which make it possible.
>> +
>> +1. Simplified Problem Definition
>> +--------------------------------
>> +
>> +Assume we have a platform which boots using following simplified device tree.
>> +
>> +---- foo.dts -----------------------------------------------------------------
>> + /* FOO platform */
>> + / {
>> + compatible = "corp,foo";
>> +
>> + /* shared resources */
>> + res: res {
>> + };
>> +
>> + /* On chip peripherals */
>> + ocp: ocp {
>> + /* peripherals that are always instantiated */
>> + peripheral1 { ... };
>> + };
>> + };
>> +---- foo.dts -----------------------------------------------------------------
>> +
>> +We have a number of peripherals that after probing (using some undefined method)
>> +should result in different device tree configuration.
>> +
>> +We cannot boot with this static tree because due to the configuration of the
>> +foo platform there exist multiple conficting peripherals DT fragments.
>
> ^^^^^^^^^^ conflicting
>
> I assume conflicting because, for instance, the different peripherals might
> occupy the same address space, use the same interrupt, or use the same gpio.
> Mentioning that would provide a fuller picture for the neophyte.
>
Yes, thanks for bringing this to my attention. This document is heavy on the neophyte for sure.
>> +
>> +So for the bar peripheral we would have this:
>> +
>> +---- foo+bar.dts -------------------------------------------------------------
>> + /* FOO platform + bar peripheral */
>> + / {
>> + compatible = "corp,foo";
>> +
>> + /* shared resources */
>> + res: res {
>> + };
>> +
>> + /* On chip peripherals */
>> + ocp: ocp {
>> + /* peripherals that are always instantiated */
>> + peripheral1 { ... };
>> +
>> + /* bar peripheral */
>> + bar {
>> + compatible = "corp,bar";
>> + ... /* various properties and child nodes */
>> + };
>> + };
>> + };
>> +---- foo+bar.dts -------------------------------------------------------------
>> +
>> +While for the baz peripheral we would have this:
>> +
>> +---- foo+baz.dts -------------------------------------------------------------
>> + /* FOO platform + baz peripheral */
>> + / {
>> + compatible = "corp,foo";
>> +
>> + /* shared resources */
>> + res: res {
>> + /* baz resources */
>> + baz_res: res_baz { ... };
>> + };
>> +
>> + /* On chip peripherals */
>> + ocp: ocp {
>> + /* peripherals that are always instantiated */
>> + peripheral1 { ... };
>> +
>> + /* baz peripheral */
>> + baz {
>> + compatible = "corp,baz";
>> + /* reference to another point in the tree */
>> + ref-to-res = <&baz_res>;
>> + ... /* various properties and child nodes */
>> + };
>> + };
>> + };
>> +---- foo+baz.dts -------------------------------------------------------------
>> +
>> +We note that the baz case is more complicated, since the baz peripheral needs to
>> +reference another node in the DT tree.
>
> I know that there are other situations that can justify overlays, so not
> contesting the basic need with this comment. But the above situation could
> be handled in a much simpler fashion by setting the status property of each
> of the conflicting devices to disabled, then after probing setting the status
> to ok. That method removes a lot of complexity.
>
> A big driver for the concept of overlays was being able to describe different
> add on boards at run time, instead of when the base dtb was created. I think
> we have agreed that moving to a connector model instead of a raw overlay is
> the proper way to address add on boards.
>
> Can you address how an overlay can be created that will work for a board
> plugged into any of the identical sockets that is compatible with the
> board?
>
>
Yes, I will try to do so.
>> +
>> +2. Device Tree Object Format Requirements
>> +-----------------------------------------
>> +
>> +Since the device tree is used for booting a number of very different hardware
>> +platforms it is imperative that we tread very carefully.
>> +
>> +2.a) No changes to the Device Tree binary format for the base tree. We cannot
>> +modify the tree format at all and all the information we require should be
>> +encoded using device tree itself. We can add nodes that can be safely ignored
>> +by both bootloaders and the kernel. The plugin dtb's are optionally tagged
>> +with a different magic number in the header but otherwise they too are simple
>> +blobs.
>> +
>> +2.b) Changes to the DTS source format should be absolutely minimal, and should
>> +only be needed for the DT fragment definitions, and not the base boot DT.
>> +
>> +2.c) An explicit option should be used to instruct DTC to generate the required
>> +information needed for object resolution. Platforms that don't use the
>> +dynamic object format can safely ignore it.
>> +
>> +2.d) Finally, DT syntax changes should be kept to a minimum. It should be
>> +possible to express everything using the existing DT syntax.
>> +
>> +3. Implementation
>> +-----------------
>> +
>> +The basic unit of addressing in Device Tree is the phandle. Turns out it's
>> +relatively simple to extend the way phandles are generated and referenced
>> +so that it's possible to dynamically convert symbolic references (labels)
>> +to phandle values. This is a valid assumption as long as the author uses
>> +reference syntax and does not assign phandle values manually (which might
>> +be a problem with decompiled source files).
>> +
>> +We can roughly divide the operation into two steps.
>> +
>> +3.a) Compilation of the base board DTS file using the '-@' option
>> +generates a valid DT blob with an added __symbols__ node at the root node,
>> +containing a list of all nodes that are marked with a label.
>> +
>> +Using the foo.dts file above the following node will be generated;
>> +
>> +$ dtc -@ -O dtb -o foo.dtb -b 0 foo.dts
>> +$ fdtdump foo.dtb
>> +...
>> +/ {
>> + ...
>> + res {
>> + ...
>> + phandle = <0x00000001>;
>> + ...
>> + };
>> + ocp {
>> + ...
>> + phandle = <0x00000002>;
>> + ...
>> + };
>> + __symbols__ {
>> + res="/res";
>> + ocp="/ocp";
>> + };
>> +};
>> +
>> +Notice that all the nodes that had a label have been recorded, and that
>> +phandles have been generated for them.
>> +
>> +This blob can be used to boot the board normally, the __symbols__ node will
>> +be safely ignored both by the bootloader and the kernel (the only loss will
>> +be a few bytes of memory and disk space).
>> +
>> +3.b) The Device Tree fragments must be compiled with the same option but they
>> +must also have a tag (/plugin/) that allows undefined references to nodes
>> +that are not present at compilation time to be recorded so that the runtime
>> +loader can fix them.
>> +
>> +So the bar peripheral's DTS format would be of the form:
>> +
>> +/dts-v1/ /plugin/; /* allow undefined references and record them */
>> +/ {
>> + .... /* various properties for loader use; i.e. part id etc. */
>> + fragment@0 {
>> + target = <&ocp>;
>> + __overlay__ {
>> + /* bar peripheral */
>> + bar {
>> + compatible = "corp,bar";
>> + ... /* various properties and child nodes */
>> + }
>> + };
>> + };
>> +};
>
> The last version of your patches that I tested did not require specifying
> the target property, the fragment node, and the __overlay__ node. dtc
> properly created all of those items automatically. For example, I could
> go to all of the trouble of creating those items in a dts like:
>
> $ cat example_1_hand_coded.dts
> /dts-v1/;
> /plugin/;
>
> / {
>
> fragment@0 {
> target = <&am3353x_pinmux>;
>
> __overlay__ {
>
> i2c1_pins: pinmux_i2c1_pins {
> pinctrl-single,pins = <
> 0x158 0x72
> 0x15c 0x72
> >;
> };
> };
> };
>
> fragment@1 {
> target = <&i2c1>;
>
> __overlay__ {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c1_pins>;
> clock-frequency = <400000>;
> status = "okay";
>
> at24@50 {
> compatible = "at,24c256";
> pagesize = <64>;
> reg = <0x50>;
> };
> };
> };
> };
>
>
> Or I could let dtc automagically create all the special features
> (target, fragment, __overlay__) from an equivalent dts:
>
> $ cat example_1.dts
> /dts-v1/;
> /plugin/;
>
>
> &am3353x_pinmux {
> i2c1_pins: pinmux_i2c1_pins {
> pinctrl-single,pins = <
> 0x158 0x72
> 0x15c 0x72
> >;
> };
> };
>
> &i2c1 {
> #address-cells = <1>;
> #size-cells = <0>;
> pinctrl-names = "default";
> pinctrl-0 = <&i2c1_pins>;
> clock-frequency = <400000>;
> status = "okay";
>
> at24@50 {
> compatible = "at,24c256";
> pagesize = <64>;
> reg = <0x50>;
> };
> };
>
>
> I would much prefer that people never hand code the target, fragment, and
> __overlay__ in a dts source file. Exposing them at the source level adds
> complexity, confusion, and an increased chance of creating an invalid
> overlay dtb.
>
> If possible, I would prefer target, fragment, and __overlay__ not be valid
> input to dtc. It would probably be difficult to prohibit target and fragment,
> because however unlikely they are as property and node names, they are valid
> dts syntax before adding the overlay enhancements to dtc. However __overlay__
> is not a valid node name without the overlay enhancements and could remain
> invalid dts input.
>
> I prefer that target, fragment, and __overlay__ be documented as a dtb to
> target system API. In this case, for the normal developer, they are
> hidden in the binary dtb format and in the kernel (or boot loader)
> overlay framework code.
>
> I do recognize that if __overlay__ is not valid dtc input then it is not
> possible to decompile an overlay into a dts containing __overlay__ and
> then recompile that dts. This could be resolved by a more complex
> decompile that turned the overlay dtb back into the form of example_1.dts
> above.
>
> After reading to the end of this patch, I see that the simpler form of
> .dts (like example_1.dts) is also noted as "an alternative syntax to
> the expanded form for overlays".
>
>
Phew.
Let me address all that.
When I started on this the main problem was that there was no support for applying
overlays in the kernel. The original patch series for dtc is meant to support the
encoding of the required information into device tree format.
The syntax of overlays like this '&foo { };’ is a new thing that can be subject to
change.
On the last patchset I’ve split it out so that it is clear.
Now, since we’ve settled on the internal encoding format (__overlays__, target, etc)
we can tackle the syntax cases and alternative target options.
So, yes we should forbid __overlay__ to be a valid node name eventually along with
a bunch of other syntax stuff.
Having come to mind, we should see what we need for the connector format to work.
>> +
>> +Note that there's a target property that specifies the location where the
>> +contents of the overlay node will be placed, and it references the node
>> +in the foo.dts file.
>> +
>> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
>> +$ fdtdump bar.dtbo
>> +...
>> +/ {
>> + ... /* properties */
>> + fragment@0 {
>> + target = <0xffffffff>;
>> + __overlay__ {
>> + bar {
>> + compatible = "corp,bar";
>> + ... /* various properties and child nodes */
>> + }
>> + };
>> + };
>> + __fixups__ {
>> + ocp = "/fragment@0:target:0";
>> + };
>> +};
>> +
>> +No __symbols__ has been generated (no label in bar.dts).
>> +Note that the target's ocp label is undefined, so the phandle handle
>> +value is filled with the illegal value '0xffffffff', while a __fixups__
>> +node has been generated, which marks the location in the tree where
>> +the label lookup should store the runtime phandle value of the ocp node.
>> +
>> +The format of the __fixups__ node entry is
>> +
>> + <label> = "<local-full-path>:<property-name>:<offset>";
>> +
>> +<label> Is the label we're referring
>> +<local-full-path> Is the full path of the node the reference is
>> +<property-name> Is the name of the property containing the
>> + reference
>> +<offset> The offset (in bytes) of where the property's
>> + phandle value is located.
>> +
>> +Doing the same with the baz peripheral's DTS format is a little bit more
>> +involved, since baz contains references to local labels which require
>> +local fixups.
>> +
>> +/dts-v1/ /plugin/; /* allow undefined label references and record them */
>> +/ {
>> + .... /* various properties for loader use; i.e. part id etc. */
>> + fragment@0 {
>> + target = <&res>;
>> + __overlay__ {
>> + /* baz resources */
>> + baz_res: res_baz { ... };
>> + };
>> + };
>> + fragment@1 {
>> + target = <&ocp>;
>> + __overlay__ {
>> + /* baz peripheral */
>> + baz {
>> + compatible = "corp,baz";
>> + /* reference to another point in the tree */
>> + ref-to-res = <&baz_res>;
>> + ... /* various properties and child nodes */
>> + }
>> + };
>> + };
>> +};
>> +
>> +Note that &bar_res reference.
>> +
>> +$ dtc -@ -O dtb -o baz.dtbo -b 0 baz.dts
>> +$ fdtdump baz.dtbo
>> +...
>> +/ {
>> + ... /* properties */
>> + fragment@0 {
>> + target = <0xffffffff>;
>> + __overlay__ {
>> + res_baz {
>> + ....
>> + phandle = <0x00000001>;
>> + };
>> + };
>> + };
>> + fragment@1 {
>> + target = <0xffffffff>;
>> + __overlay__ {
>> + baz {
>> + compatible = "corp,baz";
>> + ... /* various properties and child nodes */
>> + ref-to-res = <0x00000001>;
>> + }
>> + };
>> + };
>> + __fixups__ {
>> + res = "/fragment@0:target:0";
>> + ocp = "/fragment@1:target:0";
>> + };
>> + __local_fixups__ {
>> + fragment@1 {
>> + __overlay__ {
>> + baz {
>> + ref-to-res = <0>;
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +This is similar to the bar case, but the reference of a local label by the
>> +baz node generates a __local_fixups__ entry that records the place that the
>> +local reference is being made. No matter how phandles are allocated from dtc
>> +the run time loader must apply an offset to each phandle in every dynamic
>> +DT object loaded. The __local_fixups__ node records the place of every
>> +local reference so that the loader can apply the offset.
>> +
>> +There is an alternative syntax to the expanded form for overlays with phandle
>> +targets which makes the format similar to the one using in .dtsi include files.
>> +
>> +So for the &ocp target example above one can simply write:
>> +
>> +/dts-v1/ /plugin/;
>> +&ocp {
>> + /* bar peripheral */
>> + bar {
>> + compatible = "corp,bar";
>> + ... /* various properties and child nodes */
>> + }
>> +};
>> +
>> +The resulting dtb object is identical.
>>
>
Regards
— Pantelis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/1] scripts: Fixing NULL pointer dereference when pos->file is NULL
From: Arnd Bergmann @ 2016-11-29 11:15 UTC (permalink / raw)
To: Maninder Singh
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
rowand.list-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
v.narang-Sze3O3UU22JBDgjK7y7TUQ, pankaj.m-Sze3O3UU22JBDgjK7y7TUQ,
ajeet.y-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <1480415699-35335-1-git-send-email-maninder1.s-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Tuesday, November 29, 2016 4:04:59 PM CET Maninder Singh wrote:
> This patch fixes NULL pointer dereference when pos->file is NULL.
>
> caught with static analysis tool.
> Signed-off-by: Maninder Singh <maninder1.s-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Vaneet Narang <v.narang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> scripts/dtc/srcpos.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/dtc/srcpos.c b/scripts/dtc/srcpos.c
> index f534c22..360fd14 100644
> --- a/scripts/dtc/srcpos.c
> +++ b/scripts/dtc/srcpos.c
> @@ -252,12 +252,11 @@ struct srcpos *
> srcpos_dump(struct srcpos *pos)
> {
> printf("file : \"%s\"\n",
> - pos->file ? (char *) pos->file : "<no file>");
> + pos->file ? pos->file->name : "<no file>");
> printf("first_line : %d\n", pos->first_line);
>
The patch looks right, but the description doesn't seem to
match the bug.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH V2 03/10] Documentation: devicetree: thermal: da9062/61 TJUNC temperature binding
From: Steve Twiss @ 2016-11-29 11:15 UTC (permalink / raw)
To: Eduardo Valentin
Cc: DEVICETREE, LINUX-KERNEL, LINUX-PM, Mark Rutland, Rob Herring,
Zhang Rui, Dmitry Torokhov, Guenter Roeck, LINUX-INPUT,
LINUX-WATCHDOG, Lee Jones, Liam Girdwood, Mark Brown,
Support Opensource, Wim Van Sebroeck
In-Reply-To: <20161129005927.GA1848-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Hi Eduardo,
On 29 November 2016 00:59, Eduardo Valentin, wrote:
> On Wed, Oct 26, 2016 at 05:56:37PM +0100, Steve Twiss wrote:
> > +Optional properties:
> > +
> > +- dlg,tjunc-temp-polling-period-ms : Specify the polling period, measured
> > + in milliseconds, between thermal zone device update checks.
>
> Can you please elaborate on why you need this chip manufacture specific
> property?
>
> Can we use the polling property of already existing in the
> Documentation/devicetree/bindings/thermal/thermal.txt
>
> See the polling properties.
[...]
> > + dlg,tjunc-temp-polling-period-ms = <3000>;
Agreed. There is a polling period built into the thermal core. I've discussed my
reasoning for this decision in answer to your other e-mail. Please refer to the
other discussion thread: https://lkml.org/lkml/2016/11/29/262
Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH V2 09/10] thermal: da9062/61: Thermal junction temperature monitoring driver
From: Steve Twiss @ 2016-11-29 11:11 UTC (permalink / raw)
To: Eduardo Valentin
Cc: LINUX-KERNEL, LINUX-PM, Zhang Rui, DEVICETREE, Dmitry Torokhov,
Guenter Roeck, LINUX-INPUT, LINUX-WATCHDOG, Lee Jones,
Liam Girdwood, Mark Brown, Mark Rutland, Rob Herring,
Support Opensource, Wim Van Sebroeck
In-Reply-To: <20161129012409.GA2813-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Hi Eduardo,
Thanks for your response.
On 29 November 2016 01:24, Eduardo Valentin, wrote:
> On Wed, Oct 26, 2016 at 05:56:39PM +0100, Steve Twiss wrote:
> > +config DA9062_THERMAL
> > + tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"
> > + depends on MFD_DA9062
> > + depends on OF
> > + help
> > + Enable this for the Dialog Semiconductor thermal sensor driver.
> > + This will report PMIC junction over-temperature for one thermal trip
> > + zone.
> > + Compatible with the DA9062 and DA9061 PMICs.
>
> Is there any hardware documentation available for this chip that can be
> pointed out?
As part of this patch set, I added a link to the the datasheet into the top-level MFD
component of the device tree binding: https://patchwork.kernel.org/patch/9426791/
You can get all the information for DA9062 and DA9061 from the patch update in that
link.
[...]
> > + /* Now read E_TEMP again: it is acting like a status bit.
> > + * If over-temperature, then this status will be true.
> > + * If not over-temperature, this status will be false.
> > + */
> > + ret = regmap_read(thermal->hw->regmap,
> > + DA9062AA_EVENT_B,
> > + &val);
> > + if (ret < 0) {
> > + dev_err(thermal->dev,
> > + "Cannot check the TJUNC temperature status\n");
> > + goto err_enable_irq;
> > + } else {
> > + if (val & DA9062AA_E_TEMP_MASK) {
> > + mutex_lock(&thermal->lock);
> > + thermal->temperature = DA9062_MILLI_CELSIUS(125);
>
> Does this mean that the chip temperature is above or equal to 125C, is
> this really a safe temperature to keep it running?
There is more information in the datasheet under the section titles "Junction
temperature supervision". The value of 125 degC comes from the "warning
temperature threshold" and the event is triggered when "the junction temperature
rises above the first threshold (TEMP_WARN), the event E_TEMP is asserted".
This suggests strictly greater than 125. However, there is no way for the chip to
know the exact temperature. The mechanism is interrupt based and triggering
only happens when the temperature rises above the threshold level.
[...]
> > +static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> > +{
> > + struct da9062_thermal *thermal = data;
> > +
> > + disable_irq_nosync(thermal->irq);
> > + schedule_delayed_work(&thermal->work, 0);
>
> Can you please elaborate a little on why you have an one shot threaded IRQ
> handler that is only disabling the IRQ then, scheduling a work with delay of 0?
>
> To my understanding, this is exactly what you get when you run your
> threaded IRQ handler, when you configure it as one shot.
>
> Have you tried simply handling the same work done in your workqueue
> handler in your threaded IRQ? That should simplify your code and get the
> same result you are expecting.
>
> If you are not getting the IRQ disabled on the threaded IRQ when
> configured as one shot, something else seams to be broken.
Over-temperature triggering is event based: an interrupt happens when the
temperature rises above 125 degC. However, this event based system changes into
a polling operation to detect when the temperature falls below the threshold
level again. This asymmetry in the chip's behaviour is the reasoning behind
why I am not using the thermal core's built-in polling functionality.
When over-temperature is reached, the interrupt from the DA9061/2 will be
repeatedly triggered. The IRQ is disabled when the first over-temperature event
happens and the status bit is polled using the work-queue until it becomes false.
After that, the IRQ is re-enabled again so a new critical temperature event can
be detected.
After the interrupt has happened, event bit for the interrupt switches into a status
bit and is used for polling and detecting when the temperature drops below the
threshold.
https://lkml.org/lkml/2016/10/20/372
https://lkml.org/lkml/2016/10/20/433
[...]
> > + thermal->zone = thermal_zone_device_register(thermal->config->name,
> > + 1, 0, thermal,
> > + &da9062_thermal_ops, NULL, 0,
> > + 0);
>
> Did you try using of-thermal?
>
> So you would avoid having specific DT properties for something that
> there is already a defined property?
In an earlier RFC, I examined a work-around by hijacking and toggling the
thermal core's built-in polling function when I needed to poll the temperature
through get_temp(). However I thought this was quite dangerous, since it would
not be using a formal thermal core interface.
https://patchwork.kernel.org/patch/9387241/
[...]
> > + ret = request_threaded_irq(thermal->irq, NULL,
> > + da9062_thermal_irq_handler,
> > + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > + "THERMAL", thermal);
>
> How about using the devm_ version?
I wanted to explicitly free_irq() before calling thermal_zone_device_unregister()
in the remove function.
Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 4/7] tests: Add overlay tests
From: Pantelis Antoniou @ 2016-11-29 11:11 UTC (permalink / raw)
To: David Gibson
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129030807.GH13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
Hi David,
> On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
>> Add a number of tests for dynamic objects/overlays.
>>
>> Re-use the original test by moving the contents to a .dtsi include
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> ---
>> tests/overlay_overlay_dtc.dts | 76 +----------------------------------
>> tests/overlay_overlay_dtc.dtsi | 83 +++++++++++++++++++++++++++++++++++++++
>> tests/overlay_overlay_new_dtc.dts | 11 ++++++
>> tests/overlay_overlay_simple.dts | 12 ++++++
>> tests/run_tests.sh | 41 +++++++++++++++++++
>> 5 files changed, 148 insertions(+), 75 deletions(-)
>> create mode 100644 tests/overlay_overlay_dtc.dtsi
>> create mode 100644 tests/overlay_overlay_new_dtc.dts
>> create mode 100644 tests/overlay_overlay_simple.dts
>>
>> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
>> index 30d2362..ca943ea 100644
>> --- a/tests/overlay_overlay_dtc.dts
>> +++ b/tests/overlay_overlay_dtc.dts
>> @@ -8,78 +8,4 @@
>> /dts-v1/;
>> /plugin/;
>>
>> -/ {
>> - /* Test that we can change an int by another */
>> - fragment@0 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - test-int-property = <43>;
>> - };
>> - };
>> -
>> - /* Test that we can replace a string by a longer one */
>> - fragment@1 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - test-str-property = "foobar";
>> - };
>> - };
>> -
>> - /* Test that we add a new property */
>> - fragment@2 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - test-str-property-2 = "foobar2";
>> - };
>> - };
>> -
>> - /* Test that we add a new node (by phandle) */
>> - fragment@3 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - new-node {
>> - new-property;
>> - };
>> - };
>> - };
>> -
>> - fragment@5 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - local: new-local-node {
>> - new-property;
>> - };
>> - };
>> - };
>> -
>> - fragment@6 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - test-phandle = <&test>, <&local>;
>> - };
>> - };
>> -
>> - fragment@7 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - test-several-phandle = <&local>, <&local>;
>> - };
>> - };
>> -
>> - fragment@8 {
>> - target = <&test>;
>> -
>> - __overlay__ {
>> - sub-test-node {
>> - new-sub-test-property;
>> - };
>> - };
>> - };
>> -};
>> +/include/ "overlay_overlay_dtc.dtsi"
>
> Don't duplicate this, just replace it with the new style. This only
> existed as essentially documentation for the libfdt overlay
> application stuff. Since the new dtc won't support the old tag
> format, there's no point having a test for it.
>
The parser now handles both tag formats just fine. I could remove support
for it if you’re willing to tackle the flak.
>> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
>> new file mode 100644
>> index 0000000..8ea8d5d
>> --- /dev/null
>> +++ b/tests/overlay_overlay_dtc.dtsi
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2016 NextThing Co
>> + * Copyright (c) 2016 Free Electrons
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +/ {
>> + /* Test that we can change an int by another */
>> + fragment@0 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + test-int-property = <43>;
>> + };
>> + };
>> +
>> + /* Test that we can replace a string by a longer one */
>> + fragment@1 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + test-str-property = "foobar";
>> + };
>> + };
>> +
>> + /* Test that we add a new property */
>> + fragment@2 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + test-str-property-2 = "foobar2";
>> + };
>> + };
>> +
>> + /* Test that we add a new node (by phandle) */
>> + fragment@3 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + new-node {
>> + new-property;
>> + };
>> + };
>> + };
>> +
>> + fragment@5 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + local: new-local-node {
>> + new-property;
>> + };
>> + };
>> + };
>> +
>> + fragment@6 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + test-phandle = <&test>, <&local>;
>> + };
>> + };
>> +
>> + fragment@7 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + test-several-phandle = <&local>, <&local>;
>> + };
>> + };
>> +
>> + fragment@8 {
>> + target = <&test>;
>> +
>> + __overlay__ {
>> + sub-test-node {
>> + new-sub-test-property;
>> + };
>> + };
>> + };
>> +};
>> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
>> new file mode 100644
>> index 0000000..14d3f54
>> --- /dev/null
>> +++ b/tests/overlay_overlay_new_dtc.dts
>> @@ -0,0 +1,11 @@
>> +/*
>> + * Copyright (c) 2016 NextThing Co
>> + * Copyright (c) 2016 Free Electrons
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +/dts-v1/ /plugin/;
>> +
>> +/include/ "overlay_overlay_dtc.dtsi"
>> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
>> new file mode 100644
>> index 0000000..8657e1e
>> --- /dev/null
>> +++ b/tests/overlay_overlay_simple.dts
>> @@ -0,0 +1,12 @@
>> +/*
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +&test {
>> + test-int-property = <43>;
>> +};
>> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
>> index e4139dd..74af0ff 100755
>> --- a/tests/run_tests.sh
>> +++ b/tests/run_tests.sh
>> @@ -181,6 +181,47 @@ overlay_tests () {
>> run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
>> run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
>> run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
>> +
>> + # new /plugin/ format
>> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
>> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
>> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
>> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
>
> Looks like you're mixing tabs and spaces here. I don't really mind
> which, but keep it consistent at least at the same indentation level.
>
Oh, sorry, I use tabs but this sections has spaces… Will fix.
>> + # test new magic option
>> + run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
>> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
>> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
>> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
>> +
>> + # test plugin source to dtb and back
>> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
>> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
>> + run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
>> +
>> + # test plugin source to dtb and back (with new magic)
>> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
>> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
>> + run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
>> +
>> + # test plugin auto-generation without using -@
>> + run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
>> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
>> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
>> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
>> +
>> + # Test suppression of fixups
>> + run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
>> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
>> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
>> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
>> +
>> + # Test generation of aliases insted of symbols
>> + run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
>> + run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
>> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
>> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
>> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
>> fi
>>
>> # Bad fixup tests
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-29 11:11 UTC (permalink / raw)
To: Ziji Hu
Cc: Gregory CLEMENT, Adrian Hunter, linux-mmc@vger.kernel.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding
In-Reply-To: <02725a0f-c061-e7f2-9a01-8975c62ab5a7@marvell.com>
[...]
>>>>
>>>
>>> Sorry that I didn't make myself clear.
>>>
>>> Our host PHY delay line consists of hundreds of sampling points.
>>> Each sampling point represents a different phase shift.
>>>
>>> In lower speed mode, our host driver will scan the delay line.
>>> It will select and test multiple sampling points, other than testing
>>> only single sampling point.
>>>
>>> If a sampling point fails to transfer cmd/data, our host driver will
>>> move to test next sampling point, until we find out a group of successful
>>> sampling points which can transfer cmd/data. At last we will select
>>> a perfect one from them.
>>
>> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
>>
>> The main problem is that the host driver has *no* knowledge about the
>> internal state of the card, as that is the responsibility of the mmc
>> core to keep track of.
>>
>> If the host driver would send a command during every update of the
>> "ios" setting, from ->set_ios(), for sure it would lead to commands
>> being sent that are "forbidden" in the current internal state of the
>> card.
>> This would lead to that the card initialization sequence fails,
>> because the card may move to an unknown internal state and the mmc
>> core would have no knowledge about what happened.
>>
>
> Yes. In theory, host layer should not initiate a command by itself.
>
> We assume that bus is idle and card is stable in Tran state, when core layer
> asks host to switch "ios".
Understand, but this is a wrong assumption. The card may very well in
another state than Tran state.
> Besides, we only select the commands which is valid in the whole procedure,
> such as CMD8 for eMMC.
> Those test commands are actually like read operations to card registers.
> The card will return to Tran state even if transfer fails. It is also easy
> for host to recover.
For example, I would recommend you to investigate in detail the
sequence for when a CMD6 command is sent to the card.
The host must *not* start sending commands from ->set_ios() during a
CMD6 sequence. For example a CMD8 is not allowed.
Moreover, due to this, I wonder if it is even possible to get this HW
to work properly.
>
>> Hmm..
>>
>> Can you specify, *exactly*, under which "ios updates" you need to
>> verify updated PHY setting changes by sending a cmd/data? Also, please
>> specify if it's enough to only test the CMD line or also DATA lines.
>>
>
> When one of the three parameters in below changes, our host driver needs
> to adjust PHY in lower speed mode.
> 1. Speed Mode (timing): like legacy mode --> HS DDR
> 2. Bus Clock: like 400KHz --> 50MHz
> 3. Bus Width: like 1-bit --> 4-bit/8-bit
>
> For eMMC, we use CMD8 to test sampling point.
> For SD, we use CMD13.
> For SDIO, currently CMD52 is used to read a register from CCCR.
> Those commands in above are all valid during the whole procedure to switch
> to high speed mode from legacy mode.
>
> It is the best case if the test command can transfer both on CMD and DAT lines.
> CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
> CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
> are also under test.
Thanks for sharing these details!
So, if possible, I would recommend you to discuss these issues with
some of the HW designers. Perhaps you can figure out an alternative
method of confirming/testing PHY setting changes? Sending commands to
the card just doesn't work well for all cases.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 11:09 UTC (permalink / raw)
To: David Gibson
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129021028.GC13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
Hi David,
> On Nov 29, 2016, at 04:10 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Mon, Nov 28, 2016 at 02:10:35PM +0200, Pantelis Antoniou wrote:
>>
>>> On Nov 28, 2016, at 06:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>>>
>>> On Fri, Nov 25, 2016 at 02:32:10PM +0200, Pantelis Antoniou wrote:
>>>> This patch enable the generation of symbols & local fixup information
>>>> for trees compiled with the -@ (--symbols) option.
>>>>
>>>> Using this patch labels in the tree and their users emit information
>>>> in __symbols__ and __local_fixups__ nodes.
>>>>
>>>> The __fixups__ node make possible the dynamic resolution of phandle
>>>> references which are present in the plugin tree but lie in the
>>>> tree that are applying the overlay against.
>>>>
>>>> While there is a new magic number for dynamic device tree/overlays blobs
>>>> it is by default enabled. Remember to use -M to generate compatible
>>>> blobs.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>> ---
>>>> Documentation/manual.txt | 25 +++++-
>>>> checks.c | 8 +-
>>>> dtc-lexer.l | 5 ++
>>>> dtc-parser.y | 50 +++++++++--
>>>> dtc.c | 39 +++++++-
>>>> dtc.h | 20 ++++-
>>>> fdtdump.c | 2 +-
>>>> flattree.c | 17 ++--
>>>> fstree.c | 2 +-
>>>> libfdt/fdt.c | 2 +-
>>>> libfdt/fdt.h | 3 +-
>>>> livetree.c | 225 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>> tests/mangle-layout.c | 7 +-
>>>> 13 files changed, 375 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>>>> index 398de32..094893b 100644
>>>> --- a/Documentation/manual.txt
>>>> +++ b/Documentation/manual.txt
>>>> @@ -119,6 +119,24 @@ Options:
>>>> Make space for <number> reserve map entries
>>>> Relevant for dtb and asm output only.
>>>>
>>>> + -@
>>>> + Generates a __symbols__ node at the root node of the resulting blob
>>>> + for any node labels used, and for any local references using phandles
>>>> + it also generates a __local_fixups__ node that tracks them.
>>>> +
>>>> + When using the /plugin/ tag all unresolved label references to
>>>> + be tracked in the __fixups__ node, making dynamic resolution possible.
>>>> +
>>>> + -A
>>>> + Generate automatically aliases for all node labels. This is similar to
>>>> + the -@ option (the __symbols__ node contain identical information) but
>>>> + the semantics are slightly different since no phandles are automatically
>>>> + generated for labeled nodes.
>>>> +
>>>> + -M
>>>> + Generate blobs with the old FDT magic number for device tree objects.
>>>> + By default blobs use the DTBO FDT magic number instead.
>>>> +
>>>> -S <bytes>
>>>> Ensure the blob at least <bytes> long, adding additional
>>>> space if needed.
>>>> @@ -146,13 +164,18 @@ Additionally, dtc performs various sanity checks on the tree.
>>>> Here is a very rough overview of the layout of a DTS source file:
>>>>
>>>>
>>>> - sourcefile: list_of_memreserve devicetree
>>>> + sourcefile: versioninfo plugindecl list_of_memreserve devicetree
>>>>
>>>> memreserve: label 'memreserve' ADDR ADDR ';'
>>>> | label 'memreserve' ADDR '-' ADDR ';'
>>>>
>>>> devicetree: '/' nodedef
>>>>
>>>> + versioninfo: '/' 'dts-v1' '/' ';'
>>>> +
>>>> + plugindecl: '/' 'plugin' '/' ';'
>>>> + | /* empty */
>>>> +
>>>> nodedef: '{' list_of_property list_of_subnode '}' ';'
>>>>
>>>> property: label PROPNAME '=' propdata ';'
>>>> diff --git a/checks.c b/checks.c
>>>> index 2bd27a4..4292f4b 100644
>>>> --- a/checks.c
>>>> +++ b/checks.c
>>>> @@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi,
>>>>
>>>> refnode = get_node_by_ref(dt, m->ref);
>>>> if (! refnode) {
>>>> - FAIL(c, "Reference to non-existent node or label \"%s\"\n",
>>>> - m->ref);
>>>> + if (!(bi->versionflags & VF_PLUGIN))
>>>> + FAIL(c, "Reference to non-existent node or "
>>>> + "label \"%s\"\n", m->ref);
>>>> + else /* mark the entry as unresolved */
>>>> + *((cell_t *)(prop->val.val + m->offset)) =
>>>> + cpu_to_fdt32(0xffffffff);
>>>> continue;
>>>> }
>>>>
>>>> diff --git a/dtc-lexer.l b/dtc-lexer.l
>>>> index 790fbf6..40bbc87 100644
>>>> --- a/dtc-lexer.l
>>>> +++ b/dtc-lexer.l
>>>> @@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...);
>>>> return DT_V1;
>>>> }
>>>>
>>>> +<*>"/plugin/" {
>>>> + DPRINT("Keyword: /plugin/\n");
>>>> + return DT_PLUGIN;
>>>> + }
>>>> +
>>>> <*>"/memreserve/" {
>>>> DPRINT("Keyword: /memreserve/\n");
>>>> BEGIN_DEFAULT();
>>>> diff --git a/dtc-parser.y b/dtc-parser.y
>>>> index 14aaf2e..1a1f660 100644
>>>> --- a/dtc-parser.y
>>>> +++ b/dtc-parser.y
>>>> @@ -19,6 +19,7 @@
>>>> */
>>>> %{
>>>> #include <stdio.h>
>>>> +#include <inttypes.h>
>>>>
>>>> #include "dtc.h"
>>>> #include "srcpos.h"
>>>> @@ -33,6 +34,7 @@ extern void yyerror(char const *s);
>>>>
>>>> extern struct boot_info *the_boot_info;
>>>> extern bool treesource_error;
>>>> +
>>>
>>> Extraneous whitespace change here
>>>
>>
>> OK.
>>
>>>> %}
>>>>
>>>> %union {
>>>> @@ -52,9 +54,11 @@ extern bool treesource_error;
>>>> struct node *nodelist;
>>>> struct reserve_info *re;
>>>> uint64_t integer;
>>>> + unsigned int flags;
>>>> }
>>>>
>>>> %token DT_V1
>>>> +%token DT_PLUGIN
>>>> %token DT_MEMRESERVE
>>>> %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
>>>> %token DT_BITS
>>>> @@ -71,6 +75,8 @@ extern bool treesource_error;
>>>>
>>>> %type <data> propdata
>>>> %type <data> propdataprefix
>>>> +%type <flags> versioninfo
>>>> +%type <flags> plugindecl
>>>> %type <re> memreserve
>>>> %type <re> memreserves
>>>> %type <array> arrayprefix
>>>> @@ -101,16 +107,34 @@ extern bool treesource_error;
>>>> %%
>>>>
>>>> sourcefile:
>>>> - v1tag memreserves devicetree
>>>> + versioninfo plugindecl memreserves devicetree
>>>> + {
>>>> + the_boot_info = build_boot_info($1 | $2, $3, $4,
>>>> + guess_boot_cpuid($4));
>>>> + }
>>>> + ;
>>>> +
>>>> +versioninfo:
>>>> + v1tag
>>>> {
>>>> - the_boot_info = build_boot_info($2, $3,
>>>> - guess_boot_cpuid($3));
>>>> + $$ = VF_DT_V1;
>>>> }
>>>> ;
>>>>
>>>> v1tag:
>>>> DT_V1 ';'
>>>> + | DT_V1
>>>> | DT_V1 ';' v1tag
>>>> +
>>>> +plugindecl:
>>>> + DT_PLUGIN ';'
>>>> + {
>>>> + $$ = VF_PLUGIN;
>>>> + }
>>>> + | /* empty */
>>>> + {
>>>> + $$ = 0;
>>>> + }
>>>> ;
>>>>
>>>> memreserves:
>>>> @@ -161,10 +185,19 @@ devicetree:
>>>> {
>>>> struct node *target = get_node_by_ref($1, $2);
>>>>
>>>> - if (target)
>>>> + if (target) {
>>>> merge_nodes(target, $3);
>>>> - else
>>>> - ERROR(&@2, "Label or path %s not found", $2);
>>>> + } else {
>>>> + /*
>>>> + * We rely on the rule being always:
>>>> + * versioninfo plugindecl memreserves devicetree
>>>> + * so $-1 is what we want (plugindecl)
>>>> + */
>>>> + if ($<flags>-1 & VF_PLUGIN)
>>>
>>> o_O... ok. I've never seen negative value references before. Can you
>>> provide a link to some documentation saying this is actually supported
>>> usage in bison? I wasn't able to find it when I looked.
>>>
>>
>> There is a section about inherited attributes in the flex & bison book by O’Reily.
>>
>> https://books.google.gr/books?id=3Sr1V5J9_qMC&lpg=PP1&dq=flex%20bison&hl=el&pg=PP1#v=onepage&q=flex%20bison&f=false
>>
>> There’s a direct link to the 2nd Edition of lex & yacc:
>>
>> https://books.google.gr/books?id=fMPxfWfe67EC&lpg=PA183&ots=RcRSji2NAT&dq=yacc%20inherited%20attributes&hl=el&pg=PA183#v=onepage&q=yacc%20inherited%20attributes&f=false
>
> Thanks for the link. I still think moving the fragment assembly out
> of the parser will be a better idea long term, but this does address
> the main concern I had, so it will do for now.
>
>>>> + add_orphan_node($1, $3, $2);
>>>> + else
>>>> + ERROR(&@2, "Label or path %s not found", $2);
>>>> + }
>>>> $$ = $1;
>>>> }
>>>> | devicetree DT_DEL_NODE DT_REF ';'
>>>> @@ -179,6 +212,11 @@ devicetree:
>>>>
>>>> $$ = $1;
>>>> }
>>>> + | /* empty */
>>>> + {
>>>> + /* build empty node */
>>>> + $$ = name_node(build_node(NULL, NULL), "");
>>>> + }
>>>> ;
>>>>
>>>> nodedef:
>>>> diff --git a/dtc.c b/dtc.c
>>>> index 9dcf640..06e91bc 100644
>>>> --- a/dtc.c
>>>> +++ b/dtc.c
>>>> @@ -32,6 +32,9 @@ int minsize; /* Minimum blob size */
>>>> int padsize; /* Additional padding to blob */
>>>> int alignsize; /* Additional padding to blob accroding to the alignsize */
>>>> int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */
>>>> +int symbol_fixup_support; /* enable symbols & fixup support */
>>>> +int auto_label_aliases; /* auto generate labels -> aliases */
>>>> +int no_dtbo_magic; /* use old FDT magic values for objects */
>>>>
>>>> static int is_power_of_2(int x)
>>>> {
>>>> @@ -59,7 +62,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
>>>> #define FDT_VERSION(version) _FDT_VERSION(version)
>>>> #define _FDT_VERSION(version) #version
>>>> static const char usage_synopsis[] = "dtc [options] <input file>";
>>>> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
>>>> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMhv";
>>>> static struct option const usage_long_opts[] = {
>>>> {"quiet", no_argument, NULL, 'q'},
>>>> {"in-format", a_argument, NULL, 'I'},
>>>> @@ -78,6 +81,9 @@ static struct option const usage_long_opts[] = {
>>>> {"phandle", a_argument, NULL, 'H'},
>>>> {"warning", a_argument, NULL, 'W'},
>>>> {"error", a_argument, NULL, 'E'},
>>>> + {"symbols", no_argument, NULL, '@'},
>>>> + {"auto-alias", no_argument, NULL, 'A'},
>>>> + {"no-dtbo-magic", no_argument, NULL, 'M'},
>>>> {"help", no_argument, NULL, 'h'},
>>>> {"version", no_argument, NULL, 'v'},
>>>> {NULL, no_argument, NULL, 0x0},
>>>> @@ -109,6 +115,9 @@ static const char * const usage_opts_help[] = {
>>>> "\t\tboth - Both \"linux,phandle\" and \"phandle\" properties",
>>>> "\n\tEnable/disable warnings (prefix with \"no-\")",
>>>> "\n\tEnable/disable errors (prefix with \"no-\")",
>>>> + "\n\tEnable symbols/fixup support",
>>>> + "\n\tEnable auto-alias of labels",
>>>> + "\n\tDo not use DTBO magic value for plugin objects",
>>>> "\n\tPrint this help and exit",
>>>> "\n\tPrint version and exit",
>>>> NULL,
>>>> @@ -153,7 +162,7 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>>>> fclose(f);
>>>>
>>>> magic = fdt32_to_cpu(magic);
>>>> - if (magic == FDT_MAGIC)
>>>> + if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO)
>>>> return "dtb";
>>>>
>>>> return guess_type_by_name(fname, fallback);
>>>> @@ -172,6 +181,7 @@ int main(int argc, char *argv[])
>>>> FILE *outf = NULL;
>>>> int outversion = DEFAULT_FDT_VERSION;
>>>> long long cmdline_boot_cpuid = -1;
>>>> + fdt32_t out_magic = FDT_MAGIC;
>>>>
>>>> quiet = 0;
>>>> reservenum = 0;
>>>> @@ -249,6 +259,16 @@ int main(int argc, char *argv[])
>>>> parse_checks_option(false, true, optarg);
>>>> break;
>>>>
>>>> + case '@':
>>>> + symbol_fixup_support = 1;
>>>> + break;
>>>> + case 'A':
>>>> + auto_label_aliases = 1;
>>>> + break;
>>>> + case 'M':
>>>> + no_dtbo_magic = 1;
>>>> + break;
>>>> +
>>>> case 'h':
>>>> usage(NULL);
>>>> default:
>>>> @@ -306,6 +326,14 @@ int main(int argc, char *argv[])
>>>> fill_fullpaths(bi->dt, "");
>>>> process_checks(force, bi);
>>>>
>>>> + if (auto_label_aliases)
>>>> + generate_label_tree(bi->dt, "aliases", false);
>>>> +
>>>> + if (symbol_fixup_support) {
>>>> + generate_label_tree(bi->dt, "__symbols__", true);
>>>> + generate_fixups_tree(bi->dt);
>>>
>>> Hang on.. this doesn't seem right. I thought -@ controlled the
>>> __symbols__ side (i.e. the part upon which we overlay) rather than the
>>> fixups side (the part which overlays). A dtbo could certainly have
>>> both, of course, but for base trees, wouldn't you have symbols without
>>> fixups? And should it be illegal to try to build a /plugin/ without
>>> -@?
>>
>> It does control both for now. For base trees having the fixup nodes
>> will allow us to do probe order dependency tracking in the future.
>
> Erm.. how?
>
>> For plugins we need the __symbols__ node to support stacked overlays, i.e.
>> overlays referring label that were introduced by a previous overlay.
>
> Yes, I realise that an overlay may well want __symbols__ as well. But
> they still seem conceptually different. I think -@ should control
> __symbols__ whereas /plugin/ should control __fixups__.
>
It is easily done. Although using /plugin/ as an auto-magic option does both
just fine.
>> For plugins there is no requirement for now to actually contain references to
>> be resolved. It can easily be enforced though.
>
> Sure, but I don't see the relevance of that here. You could just omit
> the __fixups__ node if there's nothing to go into them.
>
Hmm, yeah.
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Regards
— Pantelis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Mark Brown @ 2016-11-29 11:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
robh-DgEjT+Ai2ygdnm+yROfE0A,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3929994.7YhJtsl9c3@avalon>
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On Tue, Nov 29, 2016 at 11:11:03AM +0200, Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> > I thought we couldn't add mandatory properties once the device is already
> > present in DT for one or more platforms.
> You can, as long as you treat them as optional in the driver to retain
> backward compatibility. The DT bindings should document the properties
> expected from a new platform (older versions of the bindings will always be
> available in the git history).
The device probably never worked without power... note that the kernel
will substitute in dummy regulators for anything that isn't explicitly
mapped so it won't actually break anything.
> > Say, if we do make it mandatory for future additions, we would need to have
> > DT property for the supplies for the new platforms. If the regulators on
> > these boards are fixed supplies, they would be need to be modeled
> > using "regulator-fixed", possibly without any input supply. Is that
> > what you're suggesting?
> That's the idea, yes. Clock maintainers have a similar opinion regarding the
> clock bindings, where a clock that is not optional at the hardware level
> should be specified in DT even if it's always present.
> Mark, any opinion ?
It's best practice to always describe the power. The kernel will cope
if people don't but it's not unknown for drivers to discover a reason
for wanting information about their power and hard to retrofit that if
it's not been in there from the get go.
Please note that if you're going to CC me into a graphics thread there's
a good chance I will miss it, I get copied on quite a lot of graphics
related mail that's not really relevant so I often skip it. Changing
the subject line would help with that.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 10:55 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <66c7f8c5-94e9-a6ca-4402-fa0ccf2a6ac0-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 12:50 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 10:39, Pantelis Antoniou wrote:
>> Hi Phil,
>>
>>> On Nov 29, 2016, at 12:32 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>>
>>> On 29/11/2016 02:11, David Gibson wrote:
>>>> On Mon, Nov 28, 2016 at 12:24:20PM +0000, Phil Elwell wrote:
>>>>> On 28/11/2016 12:10, Pantelis Antoniou wrote:
>>>>>> For plugins we need the __symbols__ node to support stacked overlays, i.e.
>>>>>> overlays referring label that were introduced by a previous overlay.
>>>>> Although it is arguably useful to be able to refer to symbols created by
>>>>> one overlay from within another, do we really want all symbols to be
>>>>> global? Isn't there a call for a new syntax or usage pattern to indicate
>>>>> either that a symbol should be local to the overlay or, my preferred
>>>>> option, global?
>>>> So, this is back to a design question about the overlay format. As
>>>> noted in the initial discussions about possible "connector" formats, I
>>>> think we will want some sort of local symbols. But the current
>>>> overlay format with all global symbols is out there and we need to
>>>> support it.
>>> The overlay format we have does not dictate the scope of the symbols.
>>>
>>> In all implementations I know of - the Raspberry Pi loader, the current
>>> Linux kernel, the latest dtc patch set - there is a completely
>>> asymmetric relationship between the base DTB and an overlay:
>>> * the base DTB exports __symbols__ to resolve the overlays unresolved
>>> label references, as recorded by the __fixups__ node
>>> * the overlay's phandles are renumbered so as not to clash with the base
>>> tree using the __local_fixups__
>>> * the contents of the __overlay__ nodes are applied to the base tree, as
>>> directed by the "target" or "target-path” properties
>>>
>> Yes
>>
>>> The __symbols__ node of the overlay is ignored and discarded. The
>>> __fixups__ and __local_fixups__ in the base DTB (if present - the RPi
>>> dtc only generates them for /plugins/) are ignored.
>>>
>> That was a limitation that no-longer applies. Overlay symbols can be added
>> to the base tree symbol list with a small patch I have already posted.
> The fact that they can doesn't mean they necessarily should.
>>
>>> In the set of RPi overlays only one exports a global symbol, which it
>>> achieves with an overlay aimed at target-path = "/__symbols__" that adds
>>> a new symbol (in this case "i2c_gpio").
>>>
>>> If the __symbols__ in an overlay are automatically merged with the base
>>> symbols, that is a significant change in semantics which needs to be
>>> discussed.
>>>
>> You no longer need to do this anymore. Hope this helps :)
>
> Not really, no. With the current scheme we have control over the scope,
> but now there is none.
>
Manually adding symbols by targeting __symbols__ is just bad. There is absolutely
no guarantee that the symbol/fixup node(s) will still be there in following iterations
of the patches.
I am thinking of parsing them, recording the information in kernel structures and then
deleting them altogether.
> How does your patch handle duplicate symbols?
>
It doesn’t. Having duplicate global symbols is bad.
It appears you want scoping rules instead. Care to paste a concrete example?
> Phil
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v2 2/2] ARM: dts: da850-lcdk: specify the maximum pixel clock rate for tilcdc
From: Sekhar Nori @ 2016-11-29 10:53 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart
In-Reply-To: <1480335328-4010-3-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On Monday 28 November 2016 05:45 PM, Bartosz Golaszewski wrote:
> Due to memory throughput constraints any display mode for which the
> pixel clock rate exceeds the recommended value of 37500 KHz must be
> filtered out.
I think there might be more reasons than memory throughput constraints
for the reasoning behind 37.5Mhz cap on pixel clock. Why not just refer
to the datasheet section that places this constraint so we know its a
hardware restriction.
>
> Specify the max-pixelclock property for the display node for
> da850-lcdk.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> arch/arm/boot/dts/da850-lcdk.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index d864f11..1283263 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -285,6 +285,7 @@
>
> &display {
> status = "okay";
> + max-pixelclock = <37500>;
Should this not be in da850.dtsi since its an SoC imposed constraint? If
a board needs narrower constraint, it can override it. But I guess most
well designed boards will just hit the SoC constraint.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 10:50 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <D3BFA6AB-21C1-451B-ACF5-32EA5E615275-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 10:39, Pantelis Antoniou wrote:
> Hi Phil,
>
>> On Nov 29, 2016, at 12:32 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>
>> On 29/11/2016 02:11, David Gibson wrote:
>>> On Mon, Nov 28, 2016 at 12:24:20PM +0000, Phil Elwell wrote:
>>>> On 28/11/2016 12:10, Pantelis Antoniou wrote:
>>>>> For plugins we need the __symbols__ node to support stacked overlays, i.e.
>>>>> overlays referring label that were introduced by a previous overlay.
>>>> Although it is arguably useful to be able to refer to symbols created by
>>>> one overlay from within another, do we really want all symbols to be
>>>> global? Isn't there a call for a new syntax or usage pattern to indicate
>>>> either that a symbol should be local to the overlay or, my preferred
>>>> option, global?
>>> So, this is back to a design question about the overlay format. As
>>> noted in the initial discussions about possible "connector" formats, I
>>> think we will want some sort of local symbols. But the current
>>> overlay format with all global symbols is out there and we need to
>>> support it.
>> The overlay format we have does not dictate the scope of the symbols.
>>
>> In all implementations I know of - the Raspberry Pi loader, the current
>> Linux kernel, the latest dtc patch set - there is a completely
>> asymmetric relationship between the base DTB and an overlay:
>> * the base DTB exports __symbols__ to resolve the overlays unresolved
>> label references, as recorded by the __fixups__ node
>> * the overlay's phandles are renumbered so as not to clash with the base
>> tree using the __local_fixups__
>> * the contents of the __overlay__ nodes are applied to the base tree, as
>> directed by the "target" or "target-path” properties
>>
> Yes
>
>> The __symbols__ node of the overlay is ignored and discarded. The
>> __fixups__ and __local_fixups__ in the base DTB (if present - the RPi
>> dtc only generates them for /plugins/) are ignored.
>>
> That was a limitation that no-longer applies. Overlay symbols can be added
> to the base tree symbol list with a small patch I have already posted.
The fact that they can doesn't mean they necessarily should.
>
>> In the set of RPi overlays only one exports a global symbol, which it
>> achieves with an overlay aimed at target-path = "/__symbols__" that adds
>> a new symbol (in this case "i2c_gpio").
>>
>> If the __symbols__ in an overlay are automatically merged with the base
>> symbols, that is a significant change in semantics which needs to be
>> discussed.
>>
> You no longer need to do this anymore. Hope this helps :)
Not really, no. With the current scheme we have control over the scope,
but now there is none.
How does your patch handle duplicate symbols?
Phil
^ permalink raw reply
* [PATCH 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Neil Armstrong @ 2016-11-29 10:47 UTC (permalink / raw)
To: airlied-cv59FeDIM0c, khilman-rdvid1DuHRBWk0Htik3J/w,
carlo-KA+7E9HrN00dnm+yROfE0A
Cc: Neil Armstrong, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
victor.wan-LpR1jeaWuhtBDgjK7y7TUQ,
jerry.cao-LpR1jeaWuhtBDgjK7y7TUQ, Xing.Xu-LpR1jeaWuhtBDgjK7y7TUQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, daniel-/w4YWyX8dFk
In-Reply-To: <1480416469-9655-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
.../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++++++++
1 file changed, 134 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/meson/meson-drm.txt
diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
new file mode 100644
index 0000000..cf241be
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
@@ -0,0 +1,134 @@
+Amlogic Meson Display Controller
+================================
+
+The Amlogic Meson Display controller is composed of several components
+that are going to be documented below:
+
+DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
+ | vd1 _______ _____________ _________________ | |
+D |-------| |----| | | | | HDMI PLL |
+D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
+R |-------| |----| Processing | | | | |
+ | osd2 | | | |---| Enci ----------|----|-----VDAC------|
+R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
+A | osd1 | | | Blenders | | Encl ----------|----|---------------|
+M |-------|______|----|____________| |________________| | |
+___|__________________________________________________________|_______________|
+
+
+VIU: Video Input Unit
+---------------------
+
+The Video Input Unit is in charge of the pixel scanout from the DDR memory.
+It fetches the frames addresses, stride and parameters from the "Canvas" memory.
+This part is also in charge of the CSC (Colorspace Conversion).
+It can handle 2 OSD Planes and 2 Video Planes.
+
+VPP: Video Post Processing
+--------------------------
+
+The Video Post Processing is in charge of the scaling and blending of the
+various planes into a single pixel stream.
+There is a special "pre-blending" used by the video planes with a dedicated
+scaler and a "post-blending" to merge with the OSD Planes.
+The OSD planes also have a dedicated scaler for one of the OSD.
+
+VENC: Video Encoders
+--------------------
+
+The VENC is composed of the multiple pixel encoders :
+ - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
+ - ENCP : Progressive Video Encoder for HDMI
+ - ENCL : LCD LVDS Encoder
+The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
+tree and provides the scanout clock to the VPP and VIU.
+The ENCI is connected to a single VDAC for Composite Output.
+The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
+
+Device Tree Bindings:
+---------------------
+
+VPU: Video Processing Unit
+--------------------------
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+ - GXBB (S905) : "amlogic,meson-gxbb-vpu"
+ - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
+ - GXM (S912) : "amlogic,meson-gxm-vpu"
+ followed by the common "amlogic,meson-gx-vpu"
+- reg: base address and size of he following memory-mapped regions :
+ - vpu
+ - hhi
+ - dmc
+- reg-names: should contain the names of the previous memory regions
+- interrupts: should contain the VENC Vsync interrupt number
+
+- ports: A ports node with endpoint definitions as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt. The
+ second port should be the output endpoints for VENC connectors.
+
+CBVS Output
+-----------
+
+The VENC can output Composite/CVBS output via a dedicated VDAC, usage of such
+Composite output is optional.
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+ - GXBB (S905) : "amlogic,meson-gxbb-cvbs"
+ - GXL (S905X, S905D) : "amlogic,meson-gxl-cvbs"
+ - GXM (S912) : "amlogic,meson-gxm-cvbs"
+ followed by the common "amlogic,meson-gx-cvbs"
+
+- ports: A ports node with endpoint definitions as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt. The
+ first port should be the input endpoints, connected ot the VPU node.
+
+Example:
+
+venc_cvbs: venc-cvbs {
+ compatible = "amlogic,meson-gxbb-cvbs";
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ enc_cvbs_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ venc_cvbs_in_vpu: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vpu_out_venc_cvbs>;
+ };
+ };
+ };
+};
+
+vpu: vpu@d0100000 {
+ compatible = "amlogic,meson-gxbb-vpu";
+ reg = <0x0 0xd0100000 0x0 0x100000>,
+ <0x0 0xc883c000 0x0 0x1000>,
+ <0x0 0xc8838000 0x0 0x1000>;
+ reg-names = "base", "hhi", "dmc";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vpu_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vpu_out_venc_cvbs: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&venc_cvbs_in_vpu>;
+ };
+ };
+ };
+};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes
From: Neil Armstrong @ 2016-11-29 10:47 UTC (permalink / raw)
To: airlied-cv59FeDIM0c, khilman-rdvid1DuHRBWk0Htik3J/w,
carlo-KA+7E9HrN00dnm+yROfE0A
Cc: Neil Armstrong, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
victor.wan-LpR1jeaWuhtBDgjK7y7TUQ,
jerry.cao-LpR1jeaWuhtBDgjK7y7TUQ, Xing.Xu-LpR1jeaWuhtBDgjK7y7TUQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, daniel-/w4YWyX8dFk
In-Reply-To: <1480416469-9655-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
boards.
Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 46 ++++++++++++++++++++++
.../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 4 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 4 ++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 8 ++++
.../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 4 ++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 8 ++++
.../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 4 ++
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 8 ++++
8 files changed, 86 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..644d5f6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -153,6 +153,27 @@
};
};
+ venc_cvbs: venc-cvbs {
+ compatible = "amlogic,meson-gx-cvbs";
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ venc_cvbs_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ venc_cvbs_in_vpu: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vpu_out_venc_cvbs>;
+ };
+ };
+ };
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
@@ -356,5 +377,30 @@
status = "disabled";
};
};
+
+ vpu: vpu@d0100000 {
+ compatible = "amlogic,meson-gx-vpu";
+ reg = <0x0 0xd0100000 0x0 0x100000>,
+ <0x0 0xc883c000 0x0 0x1000>,
+ <0x0 0xc8838000 0x0 0x1000>;
+ reg-names = "base", "hhi", "dmc";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vpu_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vpu_out_venc_cvbs: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&venc_cvbs_in_vpu>;
+ };
+ };
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 9696820..a55d1cf 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -229,3 +229,7 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&venc_cvbs {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 5e5e2de..3c09bd1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -266,3 +266,7 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&venc_cvbs {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index ac5ad3b..1a321c8f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -506,3 +506,11 @@
<&clkc CLKID_FCLK_DIV2>;
clock-names = "core", "clkin0", "clkin1";
};
+
+&venc_cvbs {
+ compatible = "amlogic,meson-gxbb-cvbs", "amlogic,meson-gx-cvbs";
+};
+
+&vpu {
+ compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
index e99101a..2a9b46f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
@@ -203,3 +203,7 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&venc_cvbs {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 3af54dc..b60c5ce 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -299,3 +299,11 @@
<&clkc CLKID_FCLK_DIV2>;
clock-names = "core", "clkin0", "clkin1";
};
+
+&venc_cvbs {
+ compatible = "amlogic,meson-gxl-cvbs", "amlogic,meson-gx-cvbs";
+};
+
+&vpu {
+ compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index d320727..1ae2451 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -167,3 +167,7 @@
max-speed = <1000>;
};
};
+
+&venc_cvbs {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index c1974bb..fecd8c2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -112,3 +112,11 @@
};
};
};
+
+&venc_cvbs {
+ compatible = "amlogic,meson-gxm-cvbs", "amlogic,meson-gx-cvbs";
+};
+
+&vpu {
+ compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
+};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 10:39 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <b7ff53f6-6481-e3f1-e3b5-d0b04e563e83-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 12:32 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 02:11, David Gibson wrote:
>> On Mon, Nov 28, 2016 at 12:24:20PM +0000, Phil Elwell wrote:
>>> On 28/11/2016 12:10, Pantelis Antoniou wrote:
>>>> For plugins we need the __symbols__ node to support stacked overlays, i.e.
>>>> overlays referring label that were introduced by a previous overlay.
>>> Although it is arguably useful to be able to refer to symbols created by
>>> one overlay from within another, do we really want all symbols to be
>>> global? Isn't there a call for a new syntax or usage pattern to indicate
>>> either that a symbol should be local to the overlay or, my preferred
>>> option, global?
>> So, this is back to a design question about the overlay format. As
>> noted in the initial discussions about possible "connector" formats, I
>> think we will want some sort of local symbols. But the current
>> overlay format with all global symbols is out there and we need to
>> support it.
> The overlay format we have does not dictate the scope of the symbols.
>
> In all implementations I know of - the Raspberry Pi loader, the current
> Linux kernel, the latest dtc patch set - there is a completely
> asymmetric relationship between the base DTB and an overlay:
> * the base DTB exports __symbols__ to resolve the overlays unresolved
> label references, as recorded by the __fixups__ node
> * the overlay's phandles are renumbered so as not to clash with the base
> tree using the __local_fixups__
> * the contents of the __overlay__ nodes are applied to the base tree, as
> directed by the "target" or "target-path” properties
>
Yes
> The __symbols__ node of the overlay is ignored and discarded. The
> __fixups__ and __local_fixups__ in the base DTB (if present - the RPi
> dtc only generates them for /plugins/) are ignored.
>
That was a limitation that no-longer applies. Overlay symbols can be added
to the base tree symbol list with a small patch I have already posted.
> In the set of RPi overlays only one exports a global symbol, which it
> achieves with an overlay aimed at target-path = "/__symbols__" that adds
> a new symbol (in this case "i2c_gpio").
>
> If the __symbols__ in an overlay are automatically merged with the base
> symbols, that is a significant change in semantics which needs to be
> discussed.
>
You no longer need to do this anymore. Hope this helps :)
> Phil
Regards
— Pantelis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/1] scripts: Fixing NULL pointer dereference when pos->file is NULL
From: Maninder Singh @ 2016-11-29 10:34 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
rowand.list-Re5JQEeQqe8AvxtiuMwx3w
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
v.narang-Sze3O3UU22JBDgjK7y7TUQ, pankaj.m-Sze3O3UU22JBDgjK7y7TUQ,
ajeet.y-Sze3O3UU22JBDgjK7y7TUQ, Maninder Singh
This patch fixes NULL pointer dereference when pos->file is NULL.
caught with static analysis tool.
Signed-off-by: Maninder Singh <maninder1.s-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Vaneet Narang <v.narang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
scripts/dtc/srcpos.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/scripts/dtc/srcpos.c b/scripts/dtc/srcpos.c
index f534c22..360fd14 100644
--- a/scripts/dtc/srcpos.c
+++ b/scripts/dtc/srcpos.c
@@ -252,12 +252,11 @@ struct srcpos *
srcpos_dump(struct srcpos *pos)
{
printf("file : \"%s\"\n",
- pos->file ? (char *) pos->file : "<no file>");
+ pos->file ? pos->file->name : "<no file>");
printf("first_line : %d\n", pos->first_line);
printf("first_column: %d\n", pos->first_column);
printf("last_line : %d\n", pos->last_line);
printf("last_column : %d\n", pos->last_column);
- printf("file : %s\n", pos->file->name);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 12/13] net: ethernet: ti: cpts: calc mult and shift from refclk freq
From: Richard Cochran @ 2016-11-29 10:34 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok,
John Stultz, Thomas Gleixner
In-Reply-To: <20161128230337.6731-13-grygorii.strashko-l0cyMroinI0@public.gmane.org>
On Mon, Nov 28, 2016 at 05:03:36PM -0600, Grygorii Strashko wrote:
> +static void cpts_calc_mult_shift(struct cpts *cpts)
> +{
> + u64 frac, maxsec, ns;
> + u32 freq, mult, shift;
> +
> + freq = clk_get_rate(cpts->refclk);
> +
> + /* Calc the maximum number of seconds which we can run before
> + * wrapping around.
> + */
> + maxsec = cpts->cc.mask;
> + do_div(maxsec, freq);
> + if (maxsec > 600 && cpts->cc.mask > UINT_MAX)
> + maxsec = 600;
The reason for this test is not obvious. Why check cc.mask against
UINT_MAX? Please use the comment to explain it.
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 10:33 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Adrian Hunter, linux-mmc@vger.kernel.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding
In-Reply-To: <CAPDyKFqjNy2oJiHHt+8CkTaiiExFKaR0i4GXGKdOMMRX-swPRg@mail.gmail.com>
Hi Ulf,
On 2016/11/29 15:49, Ulf Hansson wrote:
> On 29 November 2016 at 03:53, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 23:16, Ulf Hansson wrote:
>>> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>>>
>>>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>>>> send commands for testing current sampling point set in our host PHY.
>>>>>>
>>>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>>>> tuning command (CMD21/CMD19).
>>>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>>>> speed modes.
>>>>>>
>>>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>>>> not available?
>>>>>>
>>>>>
>>>>> Normally the mmc host driver shouldn't have to care about what the
>>>>> card supports, as that is the responsibility of the mmc core to
>>>>> manage.
>>>>>
>>>>> The host should only need to implement the ->execute_tuning() ops,
>>>>> which gets called when the card supports tuning (CMD19/21). Does it
>>>>> make sense?
>>>>>
>>>> I think it is irrelevant to tuning procedure.
>>>>
>>>> Our host requires to adjust PHY setting after each time ios setting
>>>> (SDCLK/bus width/speed mode) is changed.
>>>> The simplified sequence is:
>>>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>>>> adjust PHY setting.
>>>> During PHY setting adjustment, out host driver has to send commands to
>>>> test current sampling point. Tuning is another independent step.
>>>
>>> For those speed modes (or other ios changes) that *don't* requires
>>> tuning, then what will you do when you send the command to confirm the
>>> change of PHY setting and it fails?
>>>
>>> My assumption is that you will fail anyway, by propagating the error
>>> to the mmc core. At least that what was my understanding from your
>>> earlier replies, right!?
>>>
>>> Then, I think there are no point having the host driver sending a
>>> command to confirm the PHY settings, as the mmc core will anyway
>>> discover if something goes wrong when the next command is sent.
>>>
>>> Please correct me if I am wrong!
>>>
>>
>> Sorry that I didn't make myself clear.
>>
>> Our host PHY delay line consists of hundreds of sampling points.
>> Each sampling point represents a different phase shift.
>>
>> In lower speed mode, our host driver will scan the delay line.
>> It will select and test multiple sampling points, other than testing
>> only single sampling point.
>>
>> If a sampling point fails to transfer cmd/data, our host driver will
>> move to test next sampling point, until we find out a group of successful
>> sampling points which can transfer cmd/data. At last we will select
>> a perfect one from them.
>
> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
>
> The main problem is that the host driver has *no* knowledge about the
> internal state of the card, as that is the responsibility of the mmc
> core to keep track of.
>
> If the host driver would send a command during every update of the
> "ios" setting, from ->set_ios(), for sure it would lead to commands
> being sent that are "forbidden" in the current internal state of the
> card.
> This would lead to that the card initialization sequence fails,
> because the card may move to an unknown internal state and the mmc
> core would have no knowledge about what happened.
>
Yes. In theory, host layer should not initiate a command by itself.
We assume that bus is idle and card is stable in Tran state, when core layer
asks host to switch "ios".
Besides, we only select the commands which is valid in the whole procedure,
such as CMD8 for eMMC.
Those test commands are actually like read operations to card registers.
The card will return to Tran state even if transfer fails. It is also easy
for host to recover.
> Hmm..
>
> Can you specify, *exactly*, under which "ios updates" you need to
> verify updated PHY setting changes by sending a cmd/data? Also, please
> specify if it's enough to only test the CMD line or also DATA lines.
>
When one of the three parameters in below changes, our host driver needs
to adjust PHY in lower speed mode.
1. Speed Mode (timing): like legacy mode --> HS DDR
2. Bus Clock: like 400KHz --> 50MHz
3. Bus Width: like 1-bit --> 4-bit/8-bit
For eMMC, we use CMD8 to test sampling point.
For SD, we use CMD13.
For SDIO, currently CMD52 is used to read a register from CCCR.
Those commands in above are all valid during the whole procedure to switch
to high speed mode from legacy mode.
It is the best case if the test command can transfer both on CMD and DAT lines.
CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
are also under test.
> Kind regards
> Uffe
>
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 10:32 UTC (permalink / raw)
To: David Gibson
Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129021131.GD13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
On 29/11/2016 02:11, David Gibson wrote:
> On Mon, Nov 28, 2016 at 12:24:20PM +0000, Phil Elwell wrote:
>> On 28/11/2016 12:10, Pantelis Antoniou wrote:
>>> For plugins we need the __symbols__ node to support stacked overlays, i.e.
>>> overlays referring label that were introduced by a previous overlay.
>> Although it is arguably useful to be able to refer to symbols created by
>> one overlay from within another, do we really want all symbols to be
>> global? Isn't there a call for a new syntax or usage pattern to indicate
>> either that a symbol should be local to the overlay or, my preferred
>> option, global?
> So, this is back to a design question about the overlay format. As
> noted in the initial discussions about possible "connector" formats, I
> think we will want some sort of local symbols. But the current
> overlay format with all global symbols is out there and we need to
> support it.
The overlay format we have does not dictate the scope of the symbols.
In all implementations I know of - the Raspberry Pi loader, the current
Linux kernel, the latest dtc patch set - there is a completely
asymmetric relationship between the base DTB and an overlay:
* the base DTB exports __symbols__ to resolve the overlays unresolved
label references, as recorded by the __fixups__ node
* the overlay's phandles are renumbered so as not to clash with the base
tree using the __local_fixups__
* the contents of the __overlay__ nodes are applied to the base tree, as
directed by the "target" or "target-path" properties
The __symbols__ node of the overlay is ignored and discarded. The
__fixups__ and __local_fixups__ in the base DTB (if present - the RPi
dtc only generates them for /plugins/) are ignored.
In the set of RPi overlays only one exports a global symbol, which it
achieves with an overlay aimed at target-path = "/__symbols__" that adds
a new symbol (in this case "i2c_gpio").
If the __symbols__ in an overlay are automatically merged with the base
symbols, that is a significant change in semantics which needs to be
discussed.
Phil
^ permalink raw reply
* [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Jean-Francois Moine @ 2016-11-29 10:18 UTC (permalink / raw)
To: Dave Airlie, Maxime Ripard, Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
This patchset series adds HDMI video support to the Allwinner
sun8i SoCs which include the display engine 2 (DE2).
The driver contains the code for the A83T and H3 SoCs, and
some H3 boards, but it could be used/extended for other SoCs
(A64, H2, H5) and boards (Banana PIs, Orange PIs).
v7:
- more explanations about the DE2 in the DT documentation
- separate patches for DT documentation (Rob Herring)
- show all properties in DT examples (Rob Herring)
- use drm_of_component_probe()
- use the index of the DE 'ports' in the DT as
the DE mixer number (no alias needed anymore)
- change some 'lcd' to 'tcon' in the DT
- add HDMI module parameter for DVI mode when screen overscan
problems
- fall back to some CEA modes in case of EDID read failure
- fix some settings (interlace) and simplify code
- fix bug in start of A83T HDMI
- fix lack of CLK_PLL_DE definition in the DT include
(Icenowy Zheng)
v6:
- remove audio support (other patchset to come)
- use DRM modeset data for HDMI configuration
(thanks to Jernej Škrabec)
- more meaningfull register names
- use a mutex for DE I/O protection
- merge DE and plane into one file
- don't activate the video hardware when video not started
(Maxime Ripard)
- remove 'type = "video" in DT graph ports
(Rob Herring)
- change the I/O accesses by #define instead of struct
(Maxime Ripard, André Przywara)
- remove pm functions (Maxime Ripard)
- set the pll-de/de clocks in the DT (Maxime Ripard)
- use platform_get_irq instead of irq_of_parse_and_map
(Maxime Ripard)
- rename sunxi to sun8i (Maxime Ripard)
- fix coding style errors (Maxime Ripard)
- subclass the drm structure in private data (Daniel Vetter)
- move drm_dev_register at end of init (Daniel Vetter)
v5:
- add overlay plane
- add audio support
- add support for the A83T
- add back the HDMI driver
- many bug fixes
v4:
- drivers/clk/sunxi/Makefile was missing (Emil Velikov)
v3:
- add the hardware cursor
- simplify and fix the DE2 init sequences
- generation for all SUNXI SoCs (Andre Przywara)
v2:
- remove the HDMI driver
- remarks from Chen-Yu Tsai and Russell King
- DT documentation added
Jean-Francois Moine (8):
drm: sun8i: Add a basic DRM driver for Allwinner DE2
drm/sun8i: Add DT bindings documentation of Allwinner DE2
drm: sun8i: add HDMI video support to A83T and H3
drm/sunxi: Add DT bindings documentation of Allwinner HDMI
clk: sunxi-ng: define the PLL DE clock
ARM: dts: sun8i-h3: add HDMI video nodes
ARM: dts: sun8i-h3: Add HDMI video to the Banana Pi M2+
ARM: dts: sun8i-h3: Add HDMI video to the Orange PI 2
.../devicetree/bindings/display/sunxi/hdmi.txt | 56 ++
.../bindings/display/sunxi/sun8i-de2.txt | 121 +++
arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 12 +
arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 12 +
arch/arm/boot/dts/sun8i-h3.dtsi | 65 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/sun8i/Kconfig | 26 +
drivers/gpu/drm/sun8i/Makefile | 9 +
drivers/gpu/drm/sun8i/de2_crtc.c | 449 +++++++++++
drivers/gpu/drm/sun8i/de2_crtc.h | 52 ++
drivers/gpu/drm/sun8i/de2_drv.c | 317 ++++++++
drivers/gpu/drm/sun8i/de2_drv.h | 48 ++
drivers/gpu/drm/sun8i/de2_hdmi.c | 440 +++++++++++
drivers/gpu/drm/sun8i/de2_hdmi.h | 51 ++
drivers/gpu/drm/sun8i/de2_hdmi_io.c | 842 +++++++++++++++++++++
drivers/gpu/drm/sun8i/de2_plane.c | 734 ++++++++++++++++++
include/dt-bindings/clock/sun8i-h3-ccu.h | 1 +
18 files changed, 3238 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
create mode 100644 Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
create mode 100644 drivers/gpu/drm/sun8i/Kconfig
create mode 100644 drivers/gpu/drm/sun8i/Makefile
create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.c
create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.h
create mode 100644 drivers/gpu/drm/sun8i/de2_drv.c
create mode 100644 drivers/gpu/drm/sun8i/de2_drv.h
create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.c
create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.h
create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi_io.c
create mode 100644 drivers/gpu/drm/sun8i/de2_plane.c
--
2.10.2
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v7 8/8] ARM: dts: sun8i-h3: Add HDMI video to the Orange PI 2
From: Jean-Francois Moine @ 2016-11-29 10:16 UTC (permalink / raw)
To: Dave Airlie, Maxime Ripard, Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cover.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>
Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 047e9e1..7712972 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -105,16 +105,28 @@
};
};
+&de {
+ status = "okay";
+};
+
&ehci1 {
status = "okay";
};
+&hdmi {
+ status = "okay";
+};
+
&ir {
pinctrl-names = "default";
pinctrl-0 = <&ir_pins_a>;
status = "okay";
};
+&tcon0 {
+ status = "okay";
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 10/13] net: ethernet: ti: cpts: drop excessive writes to CTRL and INT_EN regs
From: Richard Cochran @ 2016-11-29 10:14 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230337.6731-11-grygorii.strashko-l0cyMroinI0@public.gmane.org>
On Mon, Nov 28, 2016 at 05:03:34PM -0600, Grygorii Strashko wrote:
> CPTS module and IRQs are always enabled when CPTS is registered,
> before starting overflow check work, and disabled during
> deregistration, when overflow check work has been canceled already.
> So, It doesn't require to (re)enable CPTS module and IRQs in
> cpts_overflow_check().
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Acked-by: Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v7 7/8] ARM: dts: sun8i-h3: Add HDMI video to the Banana Pi M2+
From: Jean-Francois Moine @ 2016-11-29 10:14 UTC (permalink / raw)
To: Dave Airlie, Maxime Ripard, Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cover.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>
Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index c0c49dd..9f3e2f8 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -93,6 +93,10 @@
};
};
+&de {
+ status = "okay";
+};
+
&ehci1 {
status = "okay";
};
@@ -101,12 +105,20 @@
status = "okay";
};
+&hdmi {
+ status = "okay";
+};
+
&ir {
pinctrl-names = "default";
pinctrl-0 = <&ir_pins_a>;
status = "okay";
};
+&tcon0 {
+ status = "okay";
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javi Merino @ 2016-11-29 10:14 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <20161125082121.GB16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
On Fri, Nov 25, 2016 at 10:21:21AM +0200, Sakari Ailus wrote:
Hi Sakari,
> On Wed, Nov 23, 2016 at 04:15:11PM +0000, Javi Merino wrote:
> > On Wed, Nov 23, 2016 at 05:10:42PM +0200, Sakari Ailus wrote:
> > > Hi Javi,
> >
> > Hi Sakari,
> >
> > > On Wed, Nov 23, 2016 at 10:09:57AM +0000, Javi Merino wrote:
> > > > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > > > a devicetree overlay, its of_node pointer will be different each time
> > > > the overlay is applied. We are not interested in matching the
> > > > pointer, what we want to match is that the path is the one we are
> > > > expecting. Change to use of_node_cmp() so that we continue matching
> > > > after the overlay has been removed and reapplied.
> > > >
> > > > Cc: Mauro Carvalho Chehab <mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > Cc: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > > > Cc: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > > > Signed-off-by: Javi Merino <javi.merino-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > ---
> > > > Hi,
> > > >
> > > > I feel it is a bit of a hack, but I couldn't think of anything better.
> > > > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > > > solution.
> > >
> > > First I have to admit that I'm not an expert when it comes to DT overlays.
> > >
> > > That said, my understanding is that the sub-device and the async sub-device
> > > are supposed to point to the exactly same DT node. I wonder if there's
> > > actually anything wrong in the current code.
> > >
> > > If the overlay has changed between probing the driver for the async notifier
> > > and the async sub-device, there should be no match here, should there? The
> > > two nodes actually point to a node in a different overlay in that case.
> >
> > Overlays are parts of the devicetree that can be added and removed.
> > When the overlay is applied, the camera driver is probed and does
> > v4l2_async_register_subdev(). However, v4l2_async_belongs() fails.
> > The problem is with comparing pointers. I haven't looked at the
> > implementation of overlays in detail, but what I see is that the
> > of_node pointer changes when you remove and reapply an overlay (I
> > guess it's dynamically allocated and when you remove the overlay, it's
> > freed).
>
> The concern here which we were discussing was whether the overlay should be
> relied on having compliant configuration compared to the part which was not
> part of the overlay.
>
> As external components are involved, quite possibly also the ISP DT node
> will require changes, not just the image source (TV tuner, camera sensor
> etc.). This could be because of number of CSI-2 lanes or parallel bus width,
> for instance.
>
> I'd also be interested in having an actual driver implement support for
> removing and adding a DT overlay first so we'd see how this would actually
> work. We need both in order to be able to actually remove and add DT
> overlays _without_ unbinding the ISP driver. Otherwise it should already
> work in the current codebase.
Unfortunately, the driver I'm working on is not upstream and I can't
submit it to mainline. This patch fixes the issue for me, so I
thought it could be useful fix for the kernel.
Cheers,
Javi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox