* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 13:11 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: <c06f9906-6089-c145-3b36-c410d88c786d-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 15:11 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 13:08, Pantelis Antoniou wrote:
>> There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
>>
>> And have foo-property = FOO_VALUE; in in.dts.
> In a boot loader?
>
Ah, that why you don’t do that in the bootloader :)
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 13:11 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: <C5CD81E3-A9FF-4C23-A7A5-7E2A4E80E193-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 13:08, Pantelis Antoniou wrote:
> There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
>
> And have foo-property = FOO_VALUE; in in.dts.
In a boot loader?
--
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: Overlays and boolean properties
From: Pantelis Antoniou @ 2016-11-29 13:10 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, devicetree-u79uwXL29TY76Z2rM5mHXA,
Devicetree Compiler
In-Reply-To: <a02487bb-6f79-fe4d-8180-86375b9413b9-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 15:06 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> Boolean properties are defined as being properties with no content, that
> are true if present and false if absent. They pose a problem for DT
> overlays, since the proposed (and widely used) overlay mechanism does
> not allow for properties (or nodes) to be deleted; overlays can only
> make a false property true, so boolean properties are effectively
> monostable - once true they become immutable.
>
> The standard DT syntax includes /delete-property/ and /delete-node/
> directives that do what you would expect from their names, but that
> facility is not available to overlays. There is no FDT node that
> represents the deletion - the directives are acted on immediately - so
> we would need some extra markup - say __delete_property__ and
> __delete_node__ - to hold the names of items to be deleted.
>
> Before I take this further, does anybody have any thoughts on the idea?
>
The original patchset did support removing properties (by prefixing them with -).
I can revive that if we have consensus about the format/method.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 13:08 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: <dbcfc090-43e2-d6f8-6f35-2761bc4d3da1-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> On Nov 29, 2016, at 15:05 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 13:00, Pantelis Antoniou wrote:
>> An alias is the standard way to refer to nodes symbolically. They can be overwritten at
>> runtime without triggering an error.
> Can you give me a concrete example of how this would look?
Maybe later in the day, kinda busy right now.
>> Speaking of which, since these overlays are applied at runtime, why not build them with a script
>> and have a #define passed to the c preprocessor before compiling them?
> Because the parameters are applied at run time, not compile time.
>
There’s no difference; you can cpp -IFOO_VALUE=foo in.dts | dtc | cat >/config/foo/dtb in a nutshell.
And have foo-property = FOO_VALUE; in in.dts.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Overlays and boolean properties
From: Phil Elwell @ 2016-11-29 13:06 UTC (permalink / raw)
To: David Gibson, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA, Devicetree Compiler
Boolean properties are defined as being properties with no content, that
are true if present and false if absent. They pose a problem for DT
overlays, since the proposed (and widely used) overlay mechanism does
not allow for properties (or nodes) to be deleted; overlays can only
make a false property true, so boolean properties are effectively
monostable - once true they become immutable.
The standard DT syntax includes /delete-property/ and /delete-node/
directives that do what you would expect from their names, but that
facility is not available to overlays. There is no FDT node that
represents the deletion - the directives are acted on immediately - so
we would need some extra markup - say __delete_property__ and
__delete_node__ - to hold the names of items to be deleted.
Before I take this further, does anybody have any thoughts on the idea?
Phil
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 13:05 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: <27651F03-6E8F-4C76-A0E4-0DFBEC40277C-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 13:00, Pantelis Antoniou wrote:
> An alias is the standard way to refer to nodes symbolically. They can be overwritten at
> runtime without triggering an error.
Can you give me a concrete example of how this would look?
> Speaking of which, since these overlays are applied at runtime, why not build them with a script
> and have a #define passed to the c preprocessor before compiling them?
Because the parameters are applied at run time, not compile time.
Phil
--
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: Pantelis Antoniou @ 2016-11-29 13:00 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: <a1ba4783-2a3b-eefd-9c41-2f33524472fe-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 14:57 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 29/11/2016 12:24, Pantelis Antoniou wrote:
>> First note is that this is exactly what the portable connector is supposed to
>> do; abstract away the SoC differences.
>>
>> Second note is that it’s not the overlay application that’s having problems, it’s
>> your parameter patching method.
>>
>> The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
>> done by targeting aliases instead of node labels.
>>
>> I.e. You can apply the overlay, set an alias to the node and instead of referencing
>> the label, reference the alias.
> But the patching is done before the overlays are applied, in isolation
> from the base tree, so that they can still be used with the kernel
> configfs overlay mechanism. How do aliases (which associated symbols
> with absolute paths) help?
>
An alias is the standard way to refer to nodes symbolically. They can be overwritten at
runtime without triggering an error.
>> Again, this is a stop-gap until the portable connector is done, but what I take out
>> of this is the need for a parameterization step in which an overlay is modified before
>> it is applied according to an external parameter.
> Yes, absolutely.
>
Speaking of which, since these overlays are applied at runtime, why not build them with a script
and have a #define passed to the c preprocessor before compiling them?
It doesn’t appear to be a problem doing it this way.
> Phil
> —
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 12:57 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: <96BE1B80-0843-4981-AA2A-E89EA6A02600-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 29/11/2016 12:24, Pantelis Antoniou wrote:
> First note is that this is exactly what the portable connector is supposed to
> do; abstract away the SoC differences.
>
> Second note is that it’s not the overlay application that’s having problems, it’s
> your parameter patching method.
>
> The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
> done by targeting aliases instead of node labels.
>
> I.e. You can apply the overlay, set an alias to the node and instead of referencing
> the label, reference the alias.
But the patching is done before the overlays are applied, in isolation
from the base tree, so that they can still be used with the kernel
configfs overlay mechanism. How do aliases (which associated symbols
with absolute paths) help?
> Again, this is a stop-gap until the portable connector is done, but what I take out
> of this is the need for a parameterization step in which an overlay is modified before
> it is applied according to an external parameter.
Yes, absolutely.
Phil
^ permalink raw reply
* Re: [PATCH V5 00/10] PM / OPP: Multiple regulator support
From: Rafael J. Wysocki @ 2016-11-29 12:48 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Nishanth Menon, Stephen Boyd, Lists linaro-kernel,
Linux PM, Linux Kernel Mailing List, Vincent Guittot, Rob Herring,
Dave Gerlach, Mark Brown, devicetree@vger.kernel.org
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>
On Tue, Nov 29, 2016 at 7:36 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi,
>
> Some platforms (like TI) have complex DVFS configuration for CPU
> devices, where multiple regulators are required to be configured to
> change DVFS state of the device. This was explained well by Nishanth
> earlier [1].
>
> One of the major complaints around multiple regulators case was that the
> DT isn't responsible in any way to represent the order in which multiple
> supplies need to be programmed, before or after a frequency change. It
> was considered in this patch and such information is left for the
> platform specific OPP driver now, which can register its own
> opp_set_rate() callback with the OPP core and the OPP core will then
> call it during DVFS.
>
> The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
> and code to pass values for multiple regulators and verified that they
> are all properly read by the kernel (using debugfs interface).
>
> Dave Gerlach has already tested [2] it on the real TI platforms and it
> works well for him.
>
> This is rebased over: linux-next branch in the PM tree.
>
> V4->V5:
> - Stephen boyd had some minor review comments and gave his Reviewed-by
> tag for the rest. Only 2 patches don't have his RBY tag.
> - Individual patches contain the version history from V4 to V5.
Cool.
I'd still like to see that everyone agrees with patch [6/10] in particular.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-29 12:24 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: <ba8e2ed3-9798-3074-1167-3f6851321a25-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 29, 2016, at 14:11 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> Pantelis,
>
> On 29/11/2016 10:55, Pantelis Antoniou wrote:
>> 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.
> Remember that this is now part of the Linux kernel - it isn't something
> you can just change at will.
Since I’m the one that put it there, I’m sure I have a little bit of leverage.
>> 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?
>
> Concrete non-trivial examples are hard to come by. There are some simple
> cases where we've attached labels to __overlay__ nodes so that the
> contents can be patched by our overlay parameter mechanism - they could
> just be given unique names instead of just "frag0", "frag1" etc. I'm
> more concerned about parameterised macro-expanded overlays.
>
> Consider an overlay that defines a CAN controller on an SPI bus. We
> currently have two such overlays in the RPi tree, one for SPI 0.0 and
> one for SPI 0.1. Here's an extract from one of them:
>
> /* the interrupt pin of the can-controller */
> fragment@2 {
> target = <&gpio>;
> __overlay__ {
> can0_pins: can0_pins {
> brcm,pins = <25>;
> brcm,function = <0>; /* input */
> };
> };
> };
> ...
> fragment@4 {
> target = <&spi0>;
> __overlay__ {
> /* needed to avoid dtc warning */
> #address-cells = <1>;
> #size-cells = <0>;
> can0: mcp2515@0 {
> reg = <0>;
> compatible = "microchip,mcp2515";
> pinctrl-names = "default";
> pinctrl-0 = <&can0_pins>;
> spi-max-frequency = <10000000>;
> interrupt-parent = <&gpio>;
> interrupts = <25 0x2>;
> clocks = <&can0_osc>;
> };
> };
> };
>
> One day I'd like to merge these into a single parameterised version that
> could target any CS line on any SPI controller. This requires that any
> created node names are unique with the scope of the parent ("mcp2515@0",
> "can0_pins"), and that the name of the target label (spi0) is patched to
> select the correct SPI bus. Our existing, limited overlay parameter
> mechanism uses labels to identify properties to patch:
>
> spimaxfrequency = <&can0>,"spi-max-frequency:0";
>
> (I would have attached labels to the properties themselves, but that
> doesn't seem to work, contrary to the ePAPR spec.)
>
> If the labels that locate the node, property and label names to change
> also themselves have to be made unique then that adds an extra level of
> complexity.
>
> The parameter application is a pre-processing step before the overlay is
> merged, so there is nothing preventing me from filtering the symbols
> node before passing it on based on rules of my own choosing, but I
> wanted to make more people aware of this change.
>
First note is that this is exactly what the portable connector is supposed to
do; abstract away the SoC differences.
Second note is that it’s not the overlay application that’s having problems, it’s
your parameter patching method.
The 'spimaxfrequency = <&can0>,"spi-max-frequency:0”’ form could more easily be
done by targeting aliases instead of node labels.
I.e. You can apply the overlay, set an alias to the node and instead of referencing
the label, reference the alias.
Again, this is a stop-gap until the portable connector is done, but what I take out
of this is the need for a parameterization step in which an overlay is modified before
it is applied according to an external parameter.
> Phil
>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v10 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-29 12:11 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: <1F9EDF06-98B1-4270-AA58-1A9D9A9F9803-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Pantelis,
On 29/11/2016 10:55, Pantelis Antoniou wrote:
> 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.
Remember that this is now part of the Linux kernel - it isn't something
you can just change at will.
> 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?
Concrete non-trivial examples are hard to come by. There are some simple
cases where we've attached labels to __overlay__ nodes so that the
contents can be patched by our overlay parameter mechanism - they could
just be given unique names instead of just "frag0", "frag1" etc. I'm
more concerned about parameterised macro-expanded overlays.
Consider an overlay that defines a CAN controller on an SPI bus. We
currently have two such overlays in the RPi tree, one for SPI 0.0 and
one for SPI 0.1. Here's an extract from one of them:
/* the interrupt pin of the can-controller */
fragment@2 {
target = <&gpio>;
__overlay__ {
can0_pins: can0_pins {
brcm,pins = <25>;
brcm,function = <0>; /* input */
};
};
};
...
fragment@4 {
target = <&spi0>;
__overlay__ {
/* needed to avoid dtc warning */
#address-cells = <1>;
#size-cells = <0>;
can0: mcp2515@0 {
reg = <0>;
compatible = "microchip,mcp2515";
pinctrl-names = "default";
pinctrl-0 = <&can0_pins>;
spi-max-frequency = <10000000>;
interrupt-parent = <&gpio>;
interrupts = <25 0x2>;
clocks = <&can0_osc>;
};
};
};
One day I'd like to merge these into a single parameterised version that
could target any CS line on any SPI controller. This requires that any
created node names are unique with the scope of the parent ("mcp2515@0",
"can0_pins"), and that the name of the target label (spi0) is patched to
select the correct SPI bus. Our existing, limited overlay parameter
mechanism uses labels to identify properties to patch:
spimaxfrequency = <&can0>,"spi-max-frequency:0";
(I would have attached labels to the properties themselves, but that
doesn't seem to work, contrary to the ePAPR spec.)
If the labels that locate the node, property and label names to change
also themselves have to be made unique then that adds an extra level of
complexity.
The parameter application is a pre-processing step before the overlay is
merged, so there is nothing preventing me from filtering the symbols
node before passing it on based on rules of my own choosing, but I
wanted to make more people aware of this change.
Phil
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 12:00 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Adrian Hunter,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <CAPDyKFp=KHYogJE9WkJUYKphJhsrMfLjxxvNKmiAB+35bER4FQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Ulf,
On 2016/11/29 19:11, Ulf Hansson wrote:
> [...]
>
>>>>>
>>>>
>>>> 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.
>
Could you please provide an example that card might not be in Tran state?
It seems that card should be in Tran state after CMD6 succeed.
If CMD6 fails, mmc driver will not execute ios setting. Thus ->set_ios()
will not be called.
>> 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.
>
In my very own opinion, ->set_ios() is only executed after CMD6 sequence
succeeds, based on current mmc.c/sd.c/sdio.c.
I personally think that it should not interfere CMD6 sequence.
I'm afraid that HW cannot help and SW driver has to take care of this.
>>
>>> 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.
>
Thanks a lot for you patience.
Actually, we, including HW engineers, have been working on this for
a very long time. We also test a lot on many actual products. It is
quiet stable in real use scenarios.
I know it is still not good enough. It seems to be impossible to find
another practical and reliable solution, based on our tests.
Could you please provide some suggestions thus we can try our best to improve it
to meet your requirement?
Thank you.
Best regards,
Hu Ziji
> Kind regards
> Uffe
>
--
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] EDAC: mpc85xx: Add T2080 l2-cache support
From: Johannes Thumshirn @ 2016-11-29 11:58 UTC (permalink / raw)
To: Chris Packham
Cc: linux-edac-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Johannes Thumshirn, Borislav Petkov, Mauro Carvalho Chehab,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129022038.24737-1-chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>
On Tue, Nov 29, 2016 at 03:20:37PM +1300, Chris Packham wrote:
> The l2-cache controller on the T2080 SoC has similar capabilities to the
> others already supported by the mpc85xx_edac driver. Add it to the list
> of compatible devices.
>
> Signed-off-by: Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>
> ---
Looks good,
Acked-by: Johannes Thumshirn <jth-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
Johannes Thumshirn Storage
jthumshirn-l3A5Bk7waGM@public.gmane.org +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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 2/2] ARM: dts: da850-lcdk: specify the maximum pixel clock rate for tilcdc
From: Bartosz Golaszewski @ 2016-11-29 11:57 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Kevin Hilman,
Michael Turquette, Russell King, linux-drm, LKML, Peter Ujfalusi,
Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <4f2a02c4-062f-6faf-1024-2a8718a9701f@ti.com>
2016-11-29 11:53 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> 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@baylibre.com>
>> ---
>> 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.
>
Both issues fixed in v3.
Thanks,
Bartosz Golaszewski
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v3 2/2] ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
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>
At maximum CPU frequency of 300 MHz the maximum pixel clock frequency
is 37.5 MHz[1]. We must filter out any mode for which the calculated
pixel clock rate would exceed this value.
Specify the max-pixelclock property for the display node for
da850-lcdk.
[1] http://processors.wiki.ti.com/index.php/OMAP-L1x/C674x/AM1x_LCD_Controller_(LCDC)_Throughput_and_Optimization_Techniques
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/boot/dts/da850.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 5f4ba2e..00692d3 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -453,6 +453,7 @@
compatible = "ti,da850-tilcdc";
reg = <0x213000 0x1000>;
interrupts = <52>;
+ max-pixelclock = <37500>;
status = "disabled";
ports {
--
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 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
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