Hi Phil,
> On Nov 29, 2016, at 14:11 , Phil Elwell <phil@xxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
