[linux-sunxi] Re: [PATCH 3/4] simplefb: disable dt node upon remove

[Grant Likely]

On Wed, Aug 13, 2014 at 1:54 PM, jonsmirl at gmail.com <jonsmirl at gmail.com> wrote:
> On Wed, Aug 13, 2014 at 6:19 AM, Luc Verhaegen <libv at skynet.be> wrote:
>> On Wed, Aug 13, 2014 at 11:45:24AM +0200, Luc Verhaegen wrote:
>>> On Wed, Aug 13, 2014 at 10:23:14AM +0100, Grant Likely wrote:
>>> >
>>> > The majority of the DT code is based on the assumption of a static
>>> > tree. Pantelis has been working on being able to modify it at runtime
>>> > with overlays, but he has had to go through a lot of rework because it
>>> > is not a trivial task. When you get into modifying the DT, you need to
>>> > have a lot more understanding of the side effects to changing the
>>> > tree. The DT structure also has a lifecycle that can go beyond the
>>> > current lifecycle of the kernel. The kexec tool will extract the
>>> > current tree from the kernel, make the appropriate modifications, and
>>> > use that to boot the next kernel. Allowing any driver to modify the
>>> > tree has side effects beyond just the current kernel.
>>> >
>>> > In this specific case, it will interact badly with the work Pantelis
>>> > is doing to make platform devices work with overlays. Modifying the
>>> > status property will cause the associated struct device to get removed
>>> > in the middle of probing a driver for that device! That will most
>>> > likely cause an oops.
>>> >
>>> > Besides, Luc straight out *said*: "...even though it has no real value
>>> > today". In what circumstance is that justification for modifying the
>>> > tree?
>>>
>>> With that sentence i meant that given the current state of things, it
>>> has no real value.
>>>
>>> It has no value currently as re-probing simplefb is not going to happen.
>>> But it's not a big leap to turn simplefb into a proper module. Not that
>>> that makes much sense, but that's never stopped anyone.
>>>
>>> To me it seemed simple, dt is what drives simplefb, so dt then also
>>> becomes responsible for making sure that simplefb or another driver does
>>> not attempt to blindly use this info again. The way this is implemented
>>> i do not care for in any way, i just knew that i could not do nothing
>>> here, given the catastrophic effect disabling the clocks has on simplefb
>>> on sunxi. Given the discussion that errupted here, i'd say that this
>>> does need some resolution, and altering the dt is going to have to be
>>> part of the solution.
>>>
>>> In any case, i will gladly drop this patch, as it is not absolutely
>>> necessary. But it should be very clear that there is no going back on
>>> this dt node after the clocks were released once.
>>>
>>> Luc Verhaegen.
>>
>> What about approaching this from the other end? U-Boot could add a
>> property named "once-only" or so.
>
> Device tree is supposed to be a static description of the hardware
> usable on all operating systems. It is the wrong mechanism for
> communicating between uboot and the kernel. Use something like atags
> or the kernel command line to tell the kernel that the console has
> already been set up.

Not accurate. While it is primarily hardware description, it is also
used for firmware communication. There is loads of precedence for
this. The /chosen node is the most significant example, but there are
other places where the tree is used to provide state. For example, the
current-speed property on UART nodes.

> The switch over from simple to KMS should not be done via a node
> add/del to the device tree either.  No one has removed the device from
> the system, the device tree should not be changing.

The simple-framebuffer binding appears to be insufficient in this
regard in that it doesn't have any linkage with the actual device
providing the framebuffer. Ideally, I would put the simple framebuffer
state directly into the video device node and use the chosen node to
point to the stdout device (probably with the stdout-path property).
Then the driver already knows it can just ignore the simple properties
because it owns the device node when it binds.

That said, simple-framebuffer as it stands is in use so we're not
going to deprecate it. I would like to see an addition that specifies
how a controller can be associated with a simple framebuffer node.

BTW, Is anyone currently using the simple framebuffer for early
console? For early console we would want to start using it well before
setting up platform devices.

g.