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

[jonsmirl at gmail.com]

On Wed, Aug 13, 2014 at 3:14 PM, Grant Likely <grant.likely at secretlab.ca> wrote:
> 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.

I do seem to recall you telling me a long time ago that those chosen
nodes were a mistake (or maybe it was Matt Sealey). I'm pretty wary of
opening to door to device trees carrying a bunch of state.  Five years
from now the DT is going to look like a Christmas tree.

>
>> 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.



-- 
Jon Smirl
jonsmirl at gmail.com