[linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code

Thu Oct 2 07:41:10 PDT 2014

On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
> On 10/02/2014 04:16 PM, jonsmirl at gmail.com wrote:
>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 03:40 PM, jonsmirl at gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 03:27 PM, jonsmirl at gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10/02/2014 02:56 PM, jonsmirl at gmail.com wrote:
>>>>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 10/02/2014 02:22 PM, jonsmirl at gmail.com wrote:
>>>>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl at gmail.com wrote:
>>>>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>>>>>>>> ...
>>>>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>>>>
>>>>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>>>>
>>>>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>>>>
>>>>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>>>>
>>>>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>>>>
>>>>>>>>>> You need both.
>>>>>>>>>>
>>>>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>>>>
>>>>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>>>>> set_console() but it's been a while.
>>>>>>>>>
>>>>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>>>>
>>>>>>>> No, that is where you get into trouble. The device specific driver has
>>>>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>>>>> as possible.
>>>>>>>
>>>>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>>>>> say I can, deadlock.
>>>>>>
>>>>>> It is certainly possible to extend an earlyframebuffer to be able to
>>>>>> run as a user space console. It is just going to turn into a
>>>>>> Frankenmonster driver with piles of device specific, special case code
>>>>>> in it.
>>>>>
>>>>> There is nothing hardware specific about a framebuffer needing some
>>>>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>>>>> that is hardware specific, but the framebuffer driver does not need to
>>>>> worry about that, it just sees a clocks property with some random clocks
>>>>> in there, and that is as generic as it gets.
>>>>>
>>>>>> I think that device specific code belongs in a device specific driver
>>>>>> and earlyframebuffer should handoff to it as soon as possible.
>>>>>>
>>>>>>>
>>>>>>> I've already explained that we not only can do that (we already have working
>>>>>>> code proving that), but also that this is something which we absolutely need:
>>>>>>>
>>>>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>>>>> is that the right module is not included in the initrd.
>>>>>>
>>>>>> A generic earlyframebuffer would show this error.
>>>>>
>>>>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>>>>> be disabled before the initrd starts, and the screen will be black from then
>>>>
>>>> I thought the clock/regulator clean up happened after initrd loading,
>>>> but maybe that is not the case.
>>>>
>>>> A cleaner solution would then be to modify the clock/regulator clean
>>>> up to happen after driver loading is finished from initrd. Deferring
>>>> until after that completes is a fixed limit, everything is sitting
>>>> there in RAM. I would not propose extending it until harddisk based
>>>> loading happens.
>>>>
>>>> So there are two ways to do this...
>>>> 1) modify things like earlyconsole to protect device specific resource
>>>> (I think this is a bad idea)
>>>
>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>> are needed, then earlyconsole can claim them, and release them on
>>> handover to the real display driver.
>
> Jon, can you please answer this ? I really really want to know why people
> think this is such a bad idea. Understanding why people think this is a bad
> idea is necessary to be able to come up with an alternative solution.

The list of resources should not be duplicated in the device tree -
once in the simplefb node and again in the real device node. Device
tree is a hardware description and it is being twisted to solve a
software issue. This problem is not limited to clocks, same problem
exists with regulators. On SGI systems this would exist with entire
bus controllers (but they are x86 based, console is not on the root
bus). This is a very messy problem and will lead to a Frankenstein
sized driver over time.

But... I think this is a red herring which is masking the real
problem. The real problem seems to be that there is no window for
loading device specific drivers before the resource clean up phase
happens. That's a real problem -- multi architecture distros are going
to have lots of loadable device specific drivers.

This leads me into thinking that the resource cleanup is not being
done at the right time. Maybe it should be part of the housekeeping
that goes on in the system idle task or something like that. I don't
work enough with the user space transition to know the right place to
move it to.

If we can get this resource clean up sorted out, the problem with
simplefb needing a protected resource list disappears. It make sense
to me that we have to have some way of loading device specific drivers
before this clean up happens.

I'll add Linus to the list, maybe he'll give us some guidance on how
to handle this.

>
>>>
>>>> 2) delay the clock/regulator cleanup until after there is a fixed
>>>> window for device specific drivers to load in. Loading from initrd is
>>>> a fixed window.
>>>
>>> As I already explained by example in another mail, this won't work:
>>>
>>> "delaying over the initrd is not helpful. Not having the real driver
>>> load for whatever reasons, is not necessarily a boot blocking event,
>>> and if it us just missing will not lead to any error messages.
>>>
>>> So the boot will continue normally with a black screen, and things are
>>> still impossible to debug."
>>>
>>> We've been down the whole delay till $random point in time thing in the
>>> past with storage enumeration, where you need to wait for say all members
>>> of a raid set to show up. The lesson learned from that is that you should
>>> not wait $random time / event, but wait for the actual storage device to
>>> show up.
>>>
>>> And this is just like that, we need to wait for the actual display driver
>>> to have loaded and taken over before "cleaning up" the clocks used by
>>> the display engine.
>>>
>>> I guess we could just delay all clock cleanup until then, not really
>>> pretty, and I still prefer to just list the clocks in the simplefb
>>> dtnode, but if this version would be acceptable to all involved, I can
>>> live with it.
>>>
>>> Mike, would a patch adding 2 calls like these to the clock core be
>>> acceptable ?  :
>>>
>>> clk_block_disable_unused()
>>> clk_unblock_disable_unused()
>>>
>>> Where an earlyconsole driver can then call clk_block_disable_unused(),
>>> which will turn any calls to clk_disable_unused into a nop.
>>
>> Is there a way to use the existing eprobe_defer system to do this?
>
> No, that is not what it is intended for (at all). If we go this way
> 2 subsystem specific calls for this is the best way to deal with this.
>
> Regards,
>
> Hans
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Jon Smirl
jonsmirl at gmail.com