* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <542A9771.2030401@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]
On Tue, Sep 30, 2014 at 01:43:45PM +0200, Hans de Goede wrote:
> Hi,
>
> On 09/30/2014 06:59 AM, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
>
> <snip>
>
> >> But sure, you can still try to point new issues, get an obvious and
> >> robust solution, and then discard the issue when the solution doesn't
> >> go your way...
> >
> > And you've already proven that you're completely unwilling to even
> > consider any other solution than what was originally proposed, so I
> > really don't see how discussing this further with you is going to be
> > productive.
>
> That is not true, we have seriously considered various other alternatives,
> as you know since you've participated in the discussion about them.
>
> And we've found them all lacking, mostly because they are 10 times as
> complicated.
>
> You've made your point that you don't like this solution quite loudly
> already, and we've all heard you. However you seem to be mostly alone in
> this. Even the clk maintainer has said that what we want to do is
> exactly how clocks are supposed to be used in dt.
>
> If you don't like this no-one is forcing you to use the clocks property
> in your own code. If it is not there, simplefb will behave exactly as
> before.
>
> Now since you're the only one very vocally against this, and a lot
> of people are in favor of this and have a need for this, can we
> please just get this merged and get this over with ?
Whatever. I no longer care.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-09-30 11:53 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
Linux Fbdev development list, DRI Development
In-Reply-To: <542A96BF.7080303@ti.com>
On Tue, Sep 30, 2014 at 1:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 06/08/14 15:52, Daniel Vetter wrote:
>> In drm/i915 we want to get at the video= cmdline modes even when we
>> don't have fbdev support enabled, so that users can always override
>> the kernel's initial mode selection.
>>
>> But that gives us a direct depency upon the parsing code in the fbdev
>> subsystem. Since it's so little code just extract these 2 functions
>> and always build them in.
>>
>> Whiel at it fix the checkpatch fail in this code.
>>
>> v2: Also move fb_mode_option. Spotted by the kbuild.
>>
>> v3: Review from Geert:
>> - Keep the old copyright notice from fb_mem.c, although I have no
>> idea what exactly applies.
>> - Only compile this when needed.
>>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> --
>>
>> I prefer if we can merge this through drm-next since we'll use it
>> there in follow-up patches.
>> -Daniel
>> ---
>> drivers/video/fbdev/Kconfig | 4 ++
>> drivers/video/fbdev/core/Makefile | 1 +
>> drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>> drivers/video/fbdev/core/fbmem.c | 92 ----------------------------
>> drivers/video/fbdev/core/modedb.c | 3 -
>> 5 files changed, 115 insertions(+), 95 deletions(-)
>> create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
>
> Sorry for late response.
>
> Looks fine for me, and I'm fine merging it via drm-next.
>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Well totally forgotten about this one here - I even forgot to drop the
"pls ack this" text from the commit message before sending the pull
request to Dave :(
Anyway, thanks for having a look.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-09-30 12:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930085429.GB4059@ulmo>
[-- Attachment #1: Type: text/plain, Size: 5963 bytes --]
On Tue, Sep 30, 2014 at 10:54:32AM +0200, Thierry Reding wrote:
> On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> > On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> > > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> > > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> [...]
> > > > > What happened in the Snow example is that regulators that were
> > > > > previously on would all of a sudden be automatically disabled on boot
> > > > > because there was now a driver that registered them with a generic
> > > > > framework.
> > > > >
> > > > > The same thing is going to happen with simplefb for your device. If you
> > > > > later realize that you need a regulator to keep the panel going, you'll
> > > > > have to add code to your firmware to populate the corresponding
> > > > > properties, otherwise the regulator will end up unused and will be
> > > > > automatically disabled. At the same time you're going to break upstream
> > > > > for all users of your old firmware because it doesn't add that property
> > > > > yet.
> > > > >
> > > > > And the same will continue to happen for every new type of resource
> > > > > you're going to add.
> > > >
> > > > Sure, we can add any resources we will need. Regulators, reset lines,
> > > > pm domains, allocated memory, but I'm not really sure this is what you
> > > > want, right?
> > >
> > > No it's not what I want. *You* want to add resource management to this
> > > driver. What I'm saying is that if we start adding clocks then we can no
> > > longer say no to resets or regulators or power domains either.
> >
> > Yes, because resource management can be more than just "keep the thing
> > enabled". It might also be about not modifying anything, just like we
> > saw for the clocks, but that might also apply to regulators voltage.
>
> We've already determined that simplefb can't do anything meaningful with
> the resources other than keep them in the status quo. It simply doesn't
> have enough knowledge to do so. It doesn't know the exact pixel clock or
> what voltage the attached panel needs.
We do agree that it doesn't care, doesn't need to know it, or doesn't
need to do anything about it, but what it needs is that they stay the
same. That means both keeping a clock or a regulator enabled, but also
preventing any other user (direct, as in a shared regulator, or
indirect, as in two clocks sharing the same parent) to change that
voltage or pixel clock.
You were trying to address the former in your patch, but you
completely ignore the second one, which is just as important.
> > And the only way I can think of to deal with that properly is to have
> > resources management in the driver.
>
> My point is that if we had a proper way to tell the kernel not to do
> anything with resources owned by firmware, then the driver wouldn't
> have to do anything with the resources.
Yes, but at least for the clocks, and I guess it might be true in some
sick way for regulators too, the fact that it's a tree doesn't make
this easy at all. If they were completely independant clocks, yeah,
sure, we could do that. But it's almost never the case, and all clocks
share parents with other at some degree.
I guess you could do it using clock flags of some sort, but that would
require traversing the clock tree for almost any operation, which
doesn't look very reasonable.
> > > > I really start to consider adding a sunxi-uboot-fb, that would just
> > > > duplicate the source code of simplefb but also taking the
> > > > clocks. Somehow, I feel like it will be easier (and definitely less of
> > > > a hack than using the generic common clock API).
> > >
> > > You're not getting it are you? What makes you think sunxi-uboot-fb is
> > > going to be any different? This isn't about a name.
> >
> > At least, we would get to do any binding and resource management we
> > want. And that's a big win.
>
> So instead of trying to understand the concerns that I've expressed and
> constructively contribute to finding a solution that works for everybody
> you'd rather go and write a driver from scratch. Way to go.
Hey, you haven't really contributed either to finding a solution to
the fact that we need not only to prevent the clocks from being
touched during the framework initcall, but also from other related
users.
> I've already said that I'm not saying strictly no to these patches, but
> what I want to see happen is some constructive discussion about whether
> we can find better ways to do it. If we can't then I'm all for merging
> these patches. Fortunately other (sub)threads have been somewhat more
> constructive and actually come up with alternatives that should make
> everyone happier.
I didn't see where you said that you were not strictly against
them. But ok.
I guess your concerns all boil down to 1) do not break DT backward
compatibility, 2) do not break what the firmware has set up, even on
older firmwares.
We got 1 covered already, but in order to cover 2, I guess we would
need both our solutions, that I don't really see as orthogonal.
How would something like adding optional clocks property, and if
found, doing the regular clk_* calls, and if not found, relying on
your solution work for you?
> If you're going to do SoC-specific bindings and resource management
> you are in fact implementing what Grant suggested in a
> subthread. You're implementing a dummy driver only for resource
> management, which isn't really a bad thing. It can serve as a
> placeholder for now until you add the real driver. And you can also
> use the simplefb driver to provide the framebuffer.
That could be an option too, but I'd rather avoid it if possible.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-09-30 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929220250.GD5599@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 553 bytes --]
On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote:
> 2) Simplefb will only have a single user: the rpi. As the only other
> users i can think of, which does not have a full driver and which does
> not have clocks automatically disabled, are discrete cards. And they do
> not really tend to happen with dt or platform devices.
I thought the goal was for other platforms to use simplefb while waiting
for the real drivers to be loaded (so you can get get console output as
early as possible from a built in driver for example)?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-09-30 12:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930124140.GC4273@sirena.org.uk>
On Tue, Sep 30, 2014 at 8:41 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote:
>
>> 2) Simplefb will only have a single user: the rpi. As the only other
>> users i can think of, which does not have a full driver and which does
>> not have clocks automatically disabled, are discrete cards. And they do
>> not really tend to happen with dt or platform devices.
>
> I thought the goal was for other platforms to use simplefb while waiting
> for the real drivers to be loaded (so you can get get console output as
> early as possible from a built in driver for example)?
That is an option that might work. Stop trying to make simplefb work
after the system is fully booted. Instead just let it run until the
clocks get shut off. That allows it to go back to being nothing but a
simple pointer to the video buffer.
Then if you want to keep your display going and don't have a KMS
driver written, whip together a device specific framebuffer driver for
your hardware that does the right thing with the clocks, etc. The
device specific framebuffer driver can load later in the boot process
so that it doesn't have to be built into the kernel. This device
specfic driver matches on a compatible string and knows what to do
with all of the device tree info.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-09-30 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930113144.GA19694@ulmo>
On 30 September 2014 13:31, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote:
>> On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
>> >> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
>> >> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
>> >> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
>> > [...]
>> >> > > > What happened in the Snow example is that regulators that were
>> >> > > > previously on would all of a sudden be automatically disabled on boot
>> >> > > > because there was now a driver that registered them with a generic
>> >> > > > framework.
>> >> > > >
>> >> > > > The same thing is going to happen with simplefb for your device. If you
>> >> > > > later realize that you need a regulator to keep the panel going, you'll
>> >> > > > have to add code to your firmware to populate the corresponding
>> >> > > > properties, otherwise the regulator will end up unused and will be
>> >> > > > automatically disabled. At the same time you're going to break upstream
>> >> > > > for all users of your old firmware because it doesn't add that property
>> >> > > > yet.
>> >> > > >
>> >> > > > And the same will continue to happen for every new type of resource
>> >> > > > you're going to add.
>> >> > >
>> >> > > Sure, we can add any resources we will need. Regulators, reset lines,
>> >> > > pm domains, allocated memory, but I'm not really sure this is what you
>> >> > > want, right?
>> >> >
>> >> > No it's not what I want. *You* want to add resource management to this
>> >> > driver. What I'm saying is that if we start adding clocks then we can no
>> >> > longer say no to resets or regulators or power domains either.
>> >>
>> >> Yes, because resource management can be more than just "keep the thing
>> >> enabled". It might also be about not modifying anything, just like we
>> >> saw for the clocks, but that might also apply to regulators voltage.
>> >
>> > We've already determined that simplefb can't do anything meaningful with
>> > the resources other than keep them in the status quo. It simply doesn't
>> > have enough knowledge to do so. It doesn't know the exact pixel clock or
>> > what voltage the attached panel needs.
>> >
>> >> And the only way I can think of to deal with that properly is to have
>> >> resources management in the driver.
>> >
>> > My point is that if we had a proper way to tell the kernel not to do
>> > anything with resources owned by firmware, then the driver wouldn't
>> > have to do anything with the resources.
>>
>> The firmware on sunxi does not own any resources whatsoever. It ceases
>> running once it executes the kernel. This is different from ACPI and
>> UEFI where you have pieces of the firmware lingering indefinitely and
>> potentially getting invoked by user pressing some button or some other
>> hardware event. It is also different from rpi where the Linux kernel
>> effectively runs in a virtual environment created by the firmware
>> hypervisor.
>
> You know all that because you of course wrote every single firmware
> implementation that does and will ever exist for sunxi. There's nothing
> keeping anyone from running UEFI on a sunxi SoC.
The existing 'firmware' or rather loader for sunxi is u-boot.
I am not saying other solutions cannot exist. I am describing the
current situation.
>
>> So on sunxi and many other ARM machines the Linux kernel is the sole
>> owner of any resources that might happen to be available on the
>> machine. There is no firmware owning them when the Linux kernel is
>> running, ever.
>
> Of course this is part of the abstraction. The idea is that the device
> is a virtual one created by firmware. Therefore firmware owns the
> resources until the virtual device has been handed over to the kernel.
>
> If you're into splitting hairs, then the simplefb device shouldn't exist
> in the first place.
Why shoudn't it?
It is properly created by u-boot and handed over to the kernel with
all the required information for the kernel to keep it running or shut
it down as it sees fit.
>
>> And we do have a proper way to tell to the kernel what these resources
>> are used for - inserting description of them into the simplefb DT
>> node. Sure the simplefb cannot manage the resources in any way and but
>> it does own them. When it is running they are in use, when it stops
>> they are free to be reclaimed by the platform driver.
>
> Yes. And again I'm not saying anything different. What I'm saying is
> that we shouldn't need to know about the resources and instead hide that
> within the firmware, for the same reason that we're already hiding the
> register programming in hardware, namely to create an abstraction that
> works irrespective of the underlying hardware.
So then hide those resources in the Linux kernel. Because if you are
into hair splitting then on sunxi currently the Linux kernel is the
firmware and u-boot is only one of the loader stages that ultimately
executes the final firmware which is Linux.
>> >> > > I really start to consider adding a sunxi-uboot-fb, that would just
>> >> > > duplicate the source code of simplefb but also taking the
>> >> > > clocks. Somehow, I feel like it will be easier (and definitely less of
>> >> > > a hack than using the generic common clock API).
>> >> >
>> >> > You're not getting it are you? What makes you think sunxi-uboot-fb is
>> >> > going to be any different? This isn't about a name.
>> >>
>> >> At least, we would get to do any binding and resource management we
>> >> want. And that's a big win.
>> >
>> > So instead of trying to understand the concerns that I've expressed and
>> > constructively contribute to finding a solution that works for everybody
>> > you'd rather go and write a driver from scratch. Way to go.
>>
>> It's the constructive thing to do when the existing driver cannot be
>> extended to work for everybody.
>
> No, it isn't. If a generic driver doesn't work for everybody then it
> isn't generic and we should fix it. Not duplicate it and add platform
> specific quirks.
What was proposed originally is not platform specific quirk.
It was a generic solution what works on every platform when you can
provide a list of resources that are needed for simplefb to run.
It's not like sunxi is the only platform that has exposed display clocks, it it?
>
>> > I've already said that I'm not saying strictly no to these patches, but
>> > what I want to see happen is some constructive discussion about whether
>> > we can find better ways to do it. If we can't then I'm all for merging
>> > these patches. Fortunately other (sub)threads have been somewhat more
>> > constructive and actually come up with alternatives that should make
>> > everyone happier.
>>
>> What are those alternatives?
>
> Sorry, you've got to do some of the work yourself. They've been
> mentioned in this thread and the one that Maxime pointed to the other
> day.
I did not notice any in this thread, sorry.
>
>> > If you're going to do SoC-specific bindings and resource management you
>> > are in fact implementing what Grant suggested in a subthread. You're
>> > implementing a dummy driver only for resource management, which isn't
>> > really a bad thing. It can serve as a placeholder for now until you add
>> > the real driver. And you can also use the simplefb driver to provide
>> > the framebuffer.
>>
>> Oh, so back to the proposal to make a driver that claims the required
>> resources and then instantiates an unextended simplefb that is
>> oblivious to the resources to be kept simple?
>
> Pretty much, yes.
>
>> Did I not propose that way back?
>
> Yes, I think you did.
>
>> Was it not already rejected?
>
> No, I don't think it was. In fact I don't think anything was really
> rejected yet, we're still in the middle of a discussion.
>
> Thierry
You personally were against it ...
Can we agree on that as acceptable solution then?
Thanks
Michal
On 28 August 2014 12:08, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 10:57:29PM +0200, Michal Suchanek wrote:
>> On 27 August 2014 17:42, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>> > So, how would the clock driver would know about which use case we're
>> > in? How would it know about which display engine is currently running?
>> > How would it know about which video output is being set?
>> >
>> > Currently, we have two separate display engines, which can each output
>> > either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
>> > one of these combinations would require different clocks. What clocks
>> > will we put in the driver? All of them?
>> >
>>
>> since simplefb cannot be extended how about adding, say, dtfb which
>> claims the resources from dt and then instantiates a simplefb once the
>> resources are claimed? That is have a dtfb which has the clocks
>> assigned and has simplefb as child dt node.
>
> I don't see how that changes anything. All you do is add another layer
> of indirection. The fundamental problem remains the same and isn't
> solved.
>
> Thierry
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-09-30 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930050923.GB29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote:
> > So long as we're ensuring that when we don't start supporting resources
> > without DT additions or at least require DT additions to actively manage
> > them (which can then include simplefb hookup) we should be fine.
> I'm not sure I understand what you mean. If we add a driver for the PMIC
> that exposes these regulators and then add a DT node for the PMIC, we'd
> still need to fix the firmware to generate the appropriate DT properties
> to allow simplefb to enable the regulators.
No, you don't. It's only if you start describing the regulators in the
PMIC in DT that you run into problems. Unconfigured regulators won't be
touched.
> So unless firmware is updated at the same time, regulators will get
> disabled because they are unused.
That won't happen unless the regulators are explicitly described, if
they are described as unused then this will of course happen.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-09-30 18:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930060312.GE29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 3390 bytes --]
On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote:
> > Not really thought this through fully yet but would having phandles to
> > the relevant devices do the job? Kind of lines up with Grant's idea of
> > having dummy drivers.
> One of the arguments that came up during the discussion of the sunxi
> patches is that simplefb is going to be used precisely because there is
> no real driver for the display part of the SoC yet and nobody knows what
> the binding will look like. So there's nothing to point at currently and
> for the same reason having a dummy driver won't work. There's simply no
> definition of what resources are needed.
You may well need to extend the binding in future for an actual driver
but from the point of view of what's going into the block it really
should just be a case of reading the datasheet and mechanically typing
that in. If we can work out how to say where the framebuffer is we
really ought to be able to work this stuff out.
> > > There may be also resets involved. Fortunately the reset framework is
> > > minimalistic enough not to care about asserting all unused resets at
> > > late_initcall. And other things like power domains may also need to be
> > > kept on.
> > We might want to do that in the future, though it's not always the case
> > that reset is the lowest power state.
> That proves my point. If we ever decide to assert resets by default
> we'll have yet another subsystem that can potentially break existing
> DTs.
OTOH given the level of breakage that's like to introduce we might just
decide not to do that...
> In the end it brings us back to the very fundamental principles of DT
> that's been causing so much pain. For things to work properly and in a
> stable way you have to get the bindings right and complete from the
> start. That is, it needs to describe every aspect of the hardware block
> and all links to other components.
Or we ned to introduce things in a conservative fashion which does cope
with backwards compatibility; it's definitely more work but it is
doable.
> > One thing that makes me a bit nervous about this approach in the context
> > of the regulator API is the frequency with which one finds shared
> > supplies. I'm not sure if it's actually a big problem in practice but
> > it makes me worry a bit. We could probably just do something like make
> > refcounting down to zero not actually disable anything for standard
> > regulators to deal with it which might be an idea anyway in the context
> > of this sort of dodge.
> Yes, that's sort of how I expected clk_ignore_unused to work. The way I
> understood it, it would cause all unused clocks to be ignored (that is
> stay enabled if they are).
> Of course as Geert pointed out in another subthread, taking this all the
> way means that we have to disable all power management because the
> firmware device may be sharing resources with other devices and which
> therefore are not unused. That's a pretty strong argument and I don't
> have a solution for that. It is only really a problem for cases where
> the firmware virtual device isn't taken over by a proper driver at some
> point, though.
Indeed, and we also run into trouble for things where we actually need
to really turn off the resource for some reason (MMC has some needs here
for example).
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mike Turquette @ 2014-09-30 21:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929135358.GC30998@ulmo>
Quoting Thierry Reding (2014-09-29 06:54:00)
> On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote:
> > On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote:
> > > > >> Plus, speaking more specifically about the clocks, that won't prevent
> > > > >> your clock to be shut down as a side effect of a later clk_disable
> > > > >> call from another driver.
> > > >
> > > > > Furthermore isn't it a bug for a driver to call clk_disable() before a
> > > > > preceding clk_enable()? There are patches being worked on that will
> > > > > enable per-user clocks and as I understand it they will specifically
> > > > > disallow drivers to disable the hardware clock if other drivers are
> > > > > still keeping them on via their own referenc.
> > > >
> > > > Calling clk_disable() preceding clk_enable() is a bug.
> > > >
> > > > Calling clk_disable() after clk_enable() will disable the clock (and
> > > > its parents)
> > > > if the clock subsystem thinks there are no other users, which is what will
> > > > happen here.
> > >
> > > Right. I'm not sure this is really applicable to this situation, though.
> >
> > It's actually very easy to do. Have a driver that probes, enables its
> > clock, fails to probe for any reason, call clk_disable in its exit
> > path. If there's no other user at that time of this particular clock
> > tree, it will be shut down. Bam. You just lost your framebuffer.
> >
> > Really, it's just that simple, and relying on the fact that some other
> > user of the same clock tree will always be their is beyond fragile.
>
> Perhaps the meaning clk_ignore_unused should be revised, then. What you
> describe isn't at all what I'd expect from such an option. And it does
> not match the description in Documentation/kernel-parameters.txt either.
From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001
From: Mike Turquette <mturquette@linaro.org>
Date: Tue, 30 Sep 2014 14:24:38 -0700
Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused
Refine the definition around clk_ignore_unused, which caused some
confusion recently on the linux-fbdev and linux-arm-kernel mailing
lists[0].
[0] http://lkml.kernel.org/r/<20140929135358.GC30998@ulmo>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
Thierry,
Please let me know if this wording makes the feature more clear.
Documentation/kernel-parameters.txt | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 10d51c2..0ce01fb 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -605,11 +605,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
See Documentation/s390/CommonIO for details.
clk_ignore_unused
[CLK]
- Keep all clocks already enabled by bootloader on,
- even if no driver has claimed them. This is useful
- for debug and development, but should not be
- needed on a platform with proper driver support.
- For more information, see Documentation/clk.txt.
+ Prevents the clock framework from automatically gating
+ clocks that have not been explicitly enabled by a Linux
+ device driver but are enabled in hardware at reset or
+ by the bootloader/firmware. Note that this does not
+ force such clocks to be always-on nor does it reserve
+ those clocks in any way. This parameter is useful for
+ debug and development, but should not be needed on a
+ platform with proper driver support. For more
+ information, see Documentation/clk.txt.
clock= [BUGS=X86-32, HW] gettimeofday clocksource override.
[Deprecated]
--
1.8.3.2
^ permalink raw reply related
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 7:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930213753.19023.17605@quantum>
[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]
On Tue, Sep 30, 2014 at 02:37:53PM -0700, Mike Turquette wrote:
> Quoting Thierry Reding (2014-09-29 06:54:00)
> > On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote:
> > > On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote:
> > > > > >> Plus, speaking more specifically about the clocks, that won't prevent
> > > > > >> your clock to be shut down as a side effect of a later clk_disable
> > > > > >> call from another driver.
> > > > >
> > > > > > Furthermore isn't it a bug for a driver to call clk_disable() before a
> > > > > > preceding clk_enable()? There are patches being worked on that will
> > > > > > enable per-user clocks and as I understand it they will specifically
> > > > > > disallow drivers to disable the hardware clock if other drivers are
> > > > > > still keeping them on via their own referenc.
> > > > >
> > > > > Calling clk_disable() preceding clk_enable() is a bug.
> > > > >
> > > > > Calling clk_disable() after clk_enable() will disable the clock (and
> > > > > its parents)
> > > > > if the clock subsystem thinks there are no other users, which is what will
> > > > > happen here.
> > > >
> > > > Right. I'm not sure this is really applicable to this situation, though.
> > >
> > > It's actually very easy to do. Have a driver that probes, enables its
> > > clock, fails to probe for any reason, call clk_disable in its exit
> > > path. If there's no other user at that time of this particular clock
> > > tree, it will be shut down. Bam. You just lost your framebuffer.
> > >
> > > Really, it's just that simple, and relying on the fact that some other
> > > user of the same clock tree will always be their is beyond fragile.
> >
> > Perhaps the meaning clk_ignore_unused should be revised, then. What you
> > describe isn't at all what I'd expect from such an option. And it does
> > not match the description in Documentation/kernel-parameters.txt either.
>
> From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001
> From: Mike Turquette <mturquette@linaro.org>
> Date: Tue, 30 Sep 2014 14:24:38 -0700
> Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused
>
> Refine the definition around clk_ignore_unused, which caused some
> confusion recently on the linux-fbdev and linux-arm-kernel mailing
> lists[0].
>
> [0] http://lkml.kernel.org/r/<20140929135358.GC30998@ulmo>
>
> Signed-off-by: Mike Turquette <mturquette@linaro.org>
> ---
> Thierry,
>
> Please let me know if this wording makes the feature more clear.
I think that's better than before, but I don't think it's accurate yet.
As pointed out by Maxime unused clock may still be disabled if it's part
of a tree and that tree is being disabled because there are no users
left.
What I had argued is that it's unexpected behaviour, because the clock
is still unused (or becomes unused again), therefore shouldn't be
disabled at that point either.
So if you want to keep the current behaviour where an unused clock can
still be disabled depending on what other users do, then I think it'd be
good to mention that as a potential caveat.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 7:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930173928.GH4273@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]
On Tue, Sep 30, 2014 at 06:39:28PM +0100, Mark Brown wrote:
> On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote:
>
> > > So long as we're ensuring that when we don't start supporting resources
> > > without DT additions or at least require DT additions to actively manage
> > > them (which can then include simplefb hookup) we should be fine.
>
> > I'm not sure I understand what you mean. If we add a driver for the PMIC
> > that exposes these regulators and then add a DT node for the PMIC, we'd
> > still need to fix the firmware to generate the appropriate DT properties
> > to allow simplefb to enable the regulators.
>
> No, you don't. It's only if you start describing the regulators in the
> PMIC in DT that you run into problems. Unconfigured regulators won't be
> touched.
Okay, that's what I meant. It seems rather odd to add a PMIC DT node but
omit the description of the regulators that it exposes. Unless the
regulators are truly unused, as in not connected to any peripherals.
> > So unless firmware is updated at the same time, regulators will get
> > disabled because they are unused.
>
> That won't happen unless the regulators are explicitly described, if
> they are described as unused then this will of course happen.
With described as unused you mean they have a node in DT, so constraints
are applied and all that, but no driver actually uses them?
The fundamental issue is that if we start describing simplefb nodes with
an incomplete set of resources then we're bound to run into problems
where it'll break once these new resources are described in the DTS. If
the simplefb node was described in the DTS then this would be less of a
problem because the resources could be added to the simplefb node at the
same time.
However given that simplefb is supposed to be generated by firmware this
is no longer possible. It will inevitably break unless you upgrade the
DTB and the firmware at the same time. And it was already decided long
ago that upgrading the firmware should never be a requirement for
keeping things working.
I don't see any way to prevent that other than ignoring the resources in
simplefb completely.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 8:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930180036.GI4273@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 6798 bytes --]
On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
> On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote:
>
> > > Not really thought this through fully yet but would having phandles to
> > > the relevant devices do the job? Kind of lines up with Grant's idea of
> > > having dummy drivers.
>
> > One of the arguments that came up during the discussion of the sunxi
> > patches is that simplefb is going to be used precisely because there is
> > no real driver for the display part of the SoC yet and nobody knows what
> > the binding will look like. So there's nothing to point at currently and
> > for the same reason having a dummy driver won't work. There's simply no
> > definition of what resources are needed.
>
> You may well need to extend the binding in future for an actual driver
> but from the point of view of what's going into the block it really
> should just be a case of reading the datasheet and mechanically typing
> that in. If we can work out how to say where the framebuffer is we
> really ought to be able to work this stuff out.
I agree from a technical point of view. However given the dynamically
generated nature of the node the problem is more of a logistical nature.
As we've seen U-Boot is being enabled to add clocks to the simplefb node
but I'm fairly certain that there's a regulator somewhere that needs to
be enabled too, be it for powering the display controller, the panel or
the backlight. I wouldn't even be surprised if there were one for each
of those. If so simplefb on this board will break when the regulators
are described in the kernel's DTS.
If we don't consider this a problem then the whole DT ABI stability
business is a farce.
> > > > There may be also resets involved. Fortunately the reset framework is
> > > > minimalistic enough not to care about asserting all unused resets at
> > > > late_initcall. And other things like power domains may also need to be
> > > > kept on.
>
> > > We might want to do that in the future, though it's not always the case
> > > that reset is the lowest power state.
>
> > That proves my point. If we ever decide to assert resets by default
> > we'll have yet another subsystem that can potentially break existing
> > DTs.
>
> OTOH given the level of breakage that's like to introduce we might just
> decide not to do that...
It might be the sensible thing to do in most cases. I think there's a
legitimate reason not to trust firmware. However in case of simplefb we
already do, so I think having a sort of flag to signal that we do trust
firmware would allow us to cope with these situation much better.
> > In the end it brings us back to the very fundamental principles of DT
> > that's been causing so much pain. For things to work properly and in a
> > stable way you have to get the bindings right and complete from the
> > start. That is, it needs to describe every aspect of the hardware block
> > and all links to other components.
>
> Or we ned to introduce things in a conservative fashion which does cope
> with backwards compatibility; it's definitely more work but it is
> doable.
Is it? I thought the only way to keep backwards compatibility was by
making any new properties optional. But if those properties add vital
information for the device to work you have to come up with a sensible
default to keep existing setups working that lack the new properties.
Doing that is not going to scale very well. It has a chance of working
for hardware-specific drivers because we may be able to derive the
default from the SoC generation or even the machine compatible. But I
don't see how it could work for something that's supposed to be generic
like simplefb.
I'm hoping that there's a better way that I don't know about, because it
would certainly make a lot of things much easier.
> > > One thing that makes me a bit nervous about this approach in the context
> > > of the regulator API is the frequency with which one finds shared
> > > supplies. I'm not sure if it's actually a big problem in practice but
> > > it makes me worry a bit. We could probably just do something like make
> > > refcounting down to zero not actually disable anything for standard
> > > regulators to deal with it which might be an idea anyway in the context
> > > of this sort of dodge.
>
> > Yes, that's sort of how I expected clk_ignore_unused to work. The way I
> > understood it, it would cause all unused clocks to be ignored (that is
> > stay enabled if they are).
>
> > Of course as Geert pointed out in another subthread, taking this all the
> > way means that we have to disable all power management because the
> > firmware device may be sharing resources with other devices and which
> > therefore are not unused. That's a pretty strong argument and I don't
> > have a solution for that. It is only really a problem for cases where
> > the firmware virtual device isn't taken over by a proper driver at some
> > point, though.
>
> Indeed, and we also run into trouble for things where we actually need
> to really turn off the resource for some reason (MMC has some needs here
> for example).
So if disabling power management wholesale isn't going to be an option,
what's the alternative? I originally proposed that the clock drivers
could be modified to not disable clocks that are known to be problematic
with simplefb. People objected to that because they thought it would be
impractical to determine which clocks are involved with display across
various boards.
Handling this in the clock driver has worked remarkably well for us on
Tegra, but perhaps that's just because Tegra is an unusually sane design
to begin with.
On the other hand you cannot do that for regulators in the drivers
because they can be used on a wide variety of boards. But at least for
regulators there's a way to handle that kind of thing in DT by marking
the regulators always-on. That allows this to become a board-integration
issue and we can actually stage things in piece by piece. For example as
long as the board doesn't support a proper display driver with all the
needed resources hooked up the regulators would be marked always-on and
allow display to keep running as set up by firmware. But once they are
properly hooked up with the real driver the always-on property can be
removed and things will keep working.
What the above doesn't handle well is seamless transition from simplefb
to a real driver, which I think is going to be a useful feature to have
and it will need to keep working for much longer than the other use-case
where simplefb is the primary framebuffer.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930180036.GI4273@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
> On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote:
[...]
> > Of course as Geert pointed out in another subthread, taking this all the
> > way means that we have to disable all power management because the
> > firmware device may be sharing resources with other devices and which
> > therefore are not unused. That's a pretty strong argument and I don't
> > have a solution for that. It is only really a problem for cases where
> > the firmware virtual device isn't taken over by a proper driver at some
> > point, though.
>
> Indeed, and we also run into trouble for things where we actually need
> to really turn off the resource for some reason (MMC has some needs here
> for example).
Perhaps an alternative would be to just keep power management going and
hope for the best. This may turn out not to be as much of a problem for
many SoCs or boards as people make it out to be.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Javier Martinez Canillas @ 2014-10-01 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001074139.GB18463@ulmo>
On Wed, Oct 1, 2014 at 9:41 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 06:39:28PM +0100, Mark Brown wrote:
>> On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote:
>> > On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote:
>>
>> > > So long as we're ensuring that when we don't start supporting resources
>> > > without DT additions or at least require DT additions to actively manage
>> > > them (which can then include simplefb hookup) we should be fine.
>>
>> > I'm not sure I understand what you mean. If we add a driver for the PMIC
>> > that exposes these regulators and then add a DT node for the PMIC, we'd
>> > still need to fix the firmware to generate the appropriate DT properties
>> > to allow simplefb to enable the regulators.
>>
>> No, you don't. It's only if you start describing the regulators in the
>> PMIC in DT that you run into problems. Unconfigured regulators won't be
>> touched.
>
> Okay, that's what I meant. It seems rather odd to add a PMIC DT node but
> omit the description of the regulators that it exposes. Unless the
> regulators are truly unused, as in not connected to any peripherals.
>
Agreed, I added similar PMIC support to other Chromebooks (Peach Pit
and Pi) DTS and for me it made totally sense to add nodes for all the
regulators that are connected to peripherals according to the board
schematic. Specially since the framework is smart enough to disable
any regulator that is not used.
After all, a DT is meant to describe the hardware, so how can possibly
be an issue to add more details about the hw in a DTS?
If something is working relying on parts of the hw on not being
described, then is essentially relying on side-effects and
implementation details which are bound to be broken anyways.
>> > So unless firmware is updated at the same time, regulators will get
>> > disabled because they are unused.
>>
>> That won't happen unless the regulators are explicitly described, if
>> they are described as unused then this will of course happen.
>
> With described as unused you mean they have a node in DT, so constraints
> are applied and all that, but no driver actually uses them?
>
Adding your resources (clock, regulators, etc) incrementally and only
when the driver for the device that use these resources is available,
will only make adding support for a new platform slower IMHO since
there will be more patches to be posted, reviewed and merged.
> The fundamental issue is that if we start describing simplefb nodes with
> an incomplete set of resources then we're bound to run into problems
> where it'll break once these new resources are described in the DTS. If
> the simplefb node was described in the DTS then this would be less of a
> problem because the resources could be added to the simplefb node at the
> same time.
>
Agreed, the assumptions made by simplefb are quite fragile so we
should either document somewhere that simplefb ignores all the
resources and that is a best effort so users should not consider the
display breaking a regression or make it robust enough so users can
expect that it will always work.
Just adding the clocks is a partial solution which I think will make
the situation even worst since it will give a false illusion of
robustness but as you said it will break anyways due other resources.
> However given that simplefb is supposed to be generated by firmware this
> is no longer possible. It will inevitably break unless you upgrade the
> DTB and the firmware at the same time. And it was already decided long
> ago that upgrading the firmware should never be a requirement for
> keeping things working.
>
AFAICT in practice most platforms' firmware do not generate the
simplefb by default. In the case of Chromebooks for example, a custom
U-boot needs to be flashed in order to have simplefb support. In fact
most people working on mainline support for Chromebooks do not use
simplefb and that is why the issue was not spot when adding the
support for clocks and regulators.
Personally I didn't even know how simplefb worked before Will reported
that his display used to work on Snow before 3.16. So I assume that
his reasonable to expect that users using simplefb are able to update
their bootloader.
Which brings a more general question about DT and backward
compatibility. Should we have backward compatibility only with the
official firmware that is ship on a device when is bought or should we
maintain backward compatibility against any firmware out there that
someone re-built and added logic to mangle the FDT before is passed to
the kernel?
Going back to simplefb, I think the fact that the simplefb is not in
the DTS is the fundamental issue here. For me, the most reasonable
approach to solve this is the one suggested by Doug Anderson. That is
to have the simplefb node in the DTS so all the references to the
resources it uses can be added in the DTS but keep the simplefb node
as status = "disabled".
The bootloader can find the simplefb node and fill all the information
about the framebuffer memory region (location and size, width, height,
format, etc) and also enable the node by setting the status to "okay"
so the simplefb driver will be probed.
If a FDT does not have a simplefb node then the boot loader can create
one (like is made for the /choosen node in most bootloaders) and make
it a best effort in that case, assuming that all the resources enabled
by the bootloader will remain enabled once the kernel boots.
This of course will require users to update their boot-loaders but as
stated above I think that is reasonable since anyone using simplefb is
using a non-official firmware anyways.
> I don't see any way to prevent that other than ignoring the resources in
> simplefb completely.
>
> Thierry
>
Best regards,
Javier
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-10-01 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001074139.GB18463@ulmo>
[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]
On Wed, Oct 01, 2014 at 09:41:40AM +0200, Thierry Reding wrote:
> On Tue, Sep 30, 2014 at 06:39:28PM +0100, Mark Brown wrote:
> > No, you don't. It's only if you start describing the regulators in the
> > PMIC in DT that you run into problems. Unconfigured regulators won't be
> > touched.
> Okay, that's what I meant. It seems rather odd to add a PMIC DT node but
> omit the description of the regulators that it exposes. Unless the
> regulators are truly unused, as in not connected to any peripherals.
Well, if one does decide to add a description of a regulator which is in
use but which hasn't yet been hooked up to users for some reason then it
needs to be marked as always on.
> > > So unless firmware is updated at the same time, regulators will get
> > > disabled because they are unused.
> > That won't happen unless the regulators are explicitly described, if
> > they are described as unused then this will of course happen.
> With described as unused you mean they have a node in DT, so constraints
> are applied and all that, but no driver actually uses them?
Yes.
> The fundamental issue is that if we start describing simplefb nodes with
> an incomplete set of resources then we're bound to run into problems
> where it'll break once these new resources are described in the DTS. If
> the simplefb node was described in the DTS then this would be less of a
> problem because the resources could be added to the simplefb node at the
> same time.
I'm not sure I follow this. If we add descriptions of new resources
then it shouldn't be hard to also add information about their use (or
that their description is incomplete) at the same time.
> However given that simplefb is supposed to be generated by firmware this
> is no longer possible. It will inevitably break unless you upgrade the
> DTB and the firmware at the same time. And it was already decided long
> ago that upgrading the firmware should never be a requirement for
> keeping things working.
> I don't see any way to prevent that other than ignoring the resources in
> simplefb completely.
One of the approaches that was being talked about was having a
placeholder in DT that the firmware fills in rather than having to
create the node from whole cloth each time, that makes life a lot
easier.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-10-01 12:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001081443.GC18463@ulmo>
[-- Attachment #1: Type: text/plain, Size: 3345 bytes --]
On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote:
> On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
> > You may well need to extend the binding in future for an actual driver
> > but from the point of view of what's going into the block it really
> > should just be a case of reading the datasheet and mechanically typing
> > that in. If we can work out how to say where the framebuffer is we
> > really ought to be able to work this stuff out.
> I agree from a technical point of view. However given the dynamically
> generated nature of the node the problem is more of a logistical nature.
> As we've seen U-Boot is being enabled to add clocks to the simplefb node
> but I'm fairly certain that there's a regulator somewhere that needs to
> be enabled too, be it for powering the display controller, the panel or
> the backlight. I wouldn't even be surprised if there were one for each
> of those. If so simplefb on this board will break when the regulators
> are described in the kernel's DTS.
> If we don't consider this a problem then the whole DT ABI stability
> business is a farce.
I think you're setting constraints on the implementation you want to see
which make it unworkable but I don't think those constraints are needed.
You're starting from the position that the DT needs to be updated without
the bootloader and that the DT must not contain any hint of simplefb as
shipped separately. That's never going to work well as far as I can see
but doesn't seem like an ABI stability issue, or at least not a
reasonable one.
Either the bootloader needs to be updated along with the DT or the DT
needs to offer the bootloader a stable interface of its own for adding
the description of what it has set up (like a default disabled node
with the FB description, I'm sure other ideas are possible). Obviously
the goal with the stable ABI is that the DT will be distributed along
with the platform.
> > > Of course as Geert pointed out in another subthread, taking this all the
> > > way means that we have to disable all power management because the
> > > firmware device may be sharing resources with other devices and which
> > > therefore are not unused. That's a pretty strong argument and I don't
> > > have a solution for that. It is only really a problem for cases where
> > > the firmware virtual device isn't taken over by a proper driver at some
> > > point, though.
> > Indeed, and we also run into trouble for things where we actually need
> > to really turn off the resource for some reason (MMC has some needs here
> > for example).
> So if disabling power management wholesale isn't going to be an option,
> what's the alternative? I originally proposed that the clock drivers
> could be modified to not disable clocks that are known to be problematic
> with simplefb. People objected to that because they thought it would be
> impractical to determine which clocks are involved with display across
> various boards.
> Handling this in the clock driver has worked remarkably well for us on
> Tegra, but perhaps that's just because Tegra is an unusually sane design
> to begin with.
It's probably more that you've just not run into the corner cases yet -
if the display is mostly driven from the standard controller on the SoC
you've got a pretty good idea what's going to be happening.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-10-01 12:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABxcv=nKErHGPRLM5yer2YnOc2VJPWLmeKgwXbXEU3r_XvsCyg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]
On Wed, Oct 01, 2014 at 01:10:46PM +0200, Javier Martinez Canillas wrote:
> On Wed, Oct 1, 2014 at 9:41 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > Okay, that's what I meant. It seems rather odd to add a PMIC DT node but
> > omit the description of the regulators that it exposes. Unless the
> > regulators are truly unused, as in not connected to any peripherals.
> Agreed, I added similar PMIC support to other Chromebooks (Peach Pit
> and Pi) DTS and for me it made totally sense to add nodes for all the
> regulators that are connected to peripherals according to the board
> schematic. Specially since the framework is smart enough to disable
> any regulator that is not used.
> After all, a DT is meant to describe the hardware, so how can possibly
> be an issue to add more details about the hw in a DTS?
> If something is working relying on parts of the hw on not being
> described, then is essentially relying on side-effects and
> implementation details which are bound to be broken anyways.
It's not a problem to describe the hardware, it's a problem to describe
the hardware inaccurately. If you add something and explicitly tell the
kernel that nothing needs it then it shouldn't be a surprise that it
gets turned off.
> > With described as unused you mean they have a node in DT, so constraints
> > are applied and all that, but no driver actually uses them?
> Adding your resources (clock, regulators, etc) incrementally and only
> when the driver for the device that use these resources is available,
> will only make adding support for a new platform slower IMHO since
> there will be more patches to be posted, reviewed and merged.
So don't do that if you're worried about it then, provide the bits of DT
that hook everything up from the start or otherwise describe things as
being in use.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001123250.GY4273@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote:
> On Wed, Oct 01, 2014 at 01:10:46PM +0200, Javier Martinez Canillas wrote:
[...]
> > Adding your resources (clock, regulators, etc) incrementally and only
> > when the driver for the device that use these resources is available,
> > will only make adding support for a new platform slower IMHO since
> > there will be more patches to be posted, reviewed and merged.
>
> So don't do that if you're worried about it then, provide the bits of DT
> that hook everything up from the start or otherwise describe things as
> being in use.
"Otherwise describe things as being in use" doesn't work for clocks for
example. And Mike already said he wasn't willing to add something like
an always-on DT property for clocks.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-10-01 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001122008.GW4273@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3618 bytes --]
On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote:
> On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote:
> > On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
>
> > > You may well need to extend the binding in future for an actual driver
> > > but from the point of view of what's going into the block it really
> > > should just be a case of reading the datasheet and mechanically typing
> > > that in. If we can work out how to say where the framebuffer is we
> > > really ought to be able to work this stuff out.
>
> > I agree from a technical point of view. However given the dynamically
> > generated nature of the node the problem is more of a logistical nature.
> > As we've seen U-Boot is being enabled to add clocks to the simplefb node
> > but I'm fairly certain that there's a regulator somewhere that needs to
> > be enabled too, be it for powering the display controller, the panel or
> > the backlight. I wouldn't even be surprised if there were one for each
> > of those. If so simplefb on this board will break when the regulators
> > are described in the kernel's DTS.
>
> > If we don't consider this a problem then the whole DT ABI stability
> > business is a farce.
>
> I think you're setting constraints on the implementation you want to see
> which make it unworkable but I don't think those constraints are needed.
> You're starting from the position that the DT needs to be updated without
> the bootloader
No, what I'm saying is that what the simplefb driver expects and what
the bootloader sets up may diverge as resource drivers are added to the
kernel. The DT /could/ be updated without the bootloader. You may only
be able to replace the DTB but not the bootloader on a given platform.
> and that the DT must not contain any hint of simplefb as
> shipped separately.
Well, I don't think it should because it describes the same resources
that the device tree node for the real device already describes. But
perhaps this is one of the cases where duplication isn't all that bad?
> That's never going to work well as far as I can see
> but doesn't seem like an ABI stability issue, or at least not a
> reasonable one.
It would work well under the assumption that the kernel wouldn't be
touching any of the resources that simplefb depends on. If that's not a
reasonable assumption then I think we can't make simplefb work the way
it's currently written.
> Either the bootloader needs to be updated along with the DT
I thought we had decided that this was one of the big no-nos. But
perhaps I'm misremembering.
> or the DT
> needs to offer the bootloader a stable interface of its own for adding
> the description of what it has set up (like a default disabled node
> with the FB description, I'm sure other ideas are possible). Obviously
> the goal with the stable ABI is that the DT will be distributed along
> with the platform.
So instead of pretending that this is in any way generic, maybe a better
idea would be to provide code in DRM/KMS drivers that is called early,
grabs all the resources as defined in the binding for the device and
then instantiates simplefb using the parsed information. Which is kind
of the stub driver that Grant had suggested.
Of course that means duplicating most of the resource handling from the
real driver into this stub driver. And it means that this part of the
driver would have to be built into the kernel and bloat it some more.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-01 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001124852.GB21733@ulmo>
On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote:
>> On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote:
>> > On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
>>
>> > > You may well need to extend the binding in future for an actual driver
>> > > but from the point of view of what's going into the block it really
>> > > should just be a case of reading the datasheet and mechanically typing
>> > > that in. If we can work out how to say where the framebuffer is we
>> > > really ought to be able to work this stuff out.
>>
>> > I agree from a technical point of view. However given the dynamically
>> > generated nature of the node the problem is more of a logistical nature.
>> > As we've seen U-Boot is being enabled to add clocks to the simplefb node
>> > but I'm fairly certain that there's a regulator somewhere that needs to
>> > be enabled too, be it for powering the display controller, the panel or
>> > the backlight. I wouldn't even be surprised if there were one for each
>> > of those. If so simplefb on this board will break when the regulators
>> > are described in the kernel's DTS.
>>
>> > If we don't consider this a problem then the whole DT ABI stability
>> > business is a farce.
>>
>> I think you're setting constraints on the implementation you want to see
>> which make it unworkable but I don't think those constraints are needed.
>> You're starting from the position that the DT needs to be updated without
>> the bootloader
>
> No, what I'm saying is that what the simplefb driver expects and what
> the bootloader sets up may diverge as resource drivers are added to the
> kernel. The DT /could/ be updated without the bootloader. You may only
> be able to replace the DTB but not the bootloader on a given platform.
simplefb should be a boot console and not survive past the boot
process. Trying to get a 'generic' console driver to survive the boot
process is not a generic problem. There are about 1,000 messages in
these threads explaining why this is not a generic problem.
All of these clock and regulator issues would go away by building a
device specific framebuffer driver. A device specific framebuffer
driver can be written in a day or two, it is far simpler than a KMS
driver. This driver would how to parse the device specific device tree
node and do the right thing with the regulators/clocks.
So simplefb is built-in and used for early boot. During the boot
process a device specific framebuffer driver loads. This device
specific driver takes over for simplefb and can become the user space
console.
If the device specific framebuffer does not get loaded, then simplefb
is going to stop working when the clocks and regulators get shut off.
But that is what should happen.
>
>> and that the DT must not contain any hint of simplefb as
>> shipped separately.
>
> Well, I don't think it should because it describes the same resources
> that the device tree node for the real device already describes. But
> perhaps this is one of the cases where duplication isn't all that bad?
>
>> That's never going to work well as far as I can see
>> but doesn't seem like an ABI stability issue, or at least not a
>> reasonable one.
>
> It would work well under the assumption that the kernel wouldn't be
> touching any of the resources that simplefb depends on. If that's not a
> reasonable assumption then I think we can't make simplefb work the way
> it's currently written.
>
>> Either the bootloader needs to be updated along with the DT
>
> I thought we had decided that this was one of the big no-nos. But
> perhaps I'm misremembering.
>
>> or the DT
>> needs to offer the bootloader a stable interface of its own for adding
>> the description of what it has set up (like a default disabled node
>> with the FB description, I'm sure other ideas are possible). Obviously
>> the goal with the stable ABI is that the DT will be distributed along
>> with the platform.
>
> So instead of pretending that this is in any way generic, maybe a better
> idea would be to provide code in DRM/KMS drivers that is called early,
> grabs all the resources as defined in the binding for the device and
> then instantiates simplefb using the parsed information. Which is kind
> of the stub driver that Grant had suggested.
>
> Of course that means duplicating most of the resource handling from the
> real driver into this stub driver. And it means that this part of the
> driver would have to be built into the kernel and bloat it some more.
>
> Thierry
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-10-01 13:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4OzeZHkkmjxe_72ykSy6wvhkEmb256-tdMB67eMVcrk7Zw@mail.gmail.com>
On 1 October 2014 15:01, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote:
>>> On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote:
>>> > On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
>>>
>>> > > You may well need to extend the binding in future for an actual driver
>>> > > but from the point of view of what's going into the block it really
>>> > > should just be a case of reading the datasheet and mechanically typing
>>> > > that in. If we can work out how to say where the framebuffer is we
>>> > > really ought to be able to work this stuff out.
>>>
>>> > I agree from a technical point of view. However given the dynamically
>>> > generated nature of the node the problem is more of a logistical nature.
>>> > As we've seen U-Boot is being enabled to add clocks to the simplefb node
>>> > but I'm fairly certain that there's a regulator somewhere that needs to
>>> > be enabled too, be it for powering the display controller, the panel or
>>> > the backlight. I wouldn't even be surprised if there were one for each
>>> > of those. If so simplefb on this board will break when the regulators
>>> > are described in the kernel's DTS.
>>>
>>> > If we don't consider this a problem then the whole DT ABI stability
>>> > business is a farce.
>>>
>>> I think you're setting constraints on the implementation you want to see
>>> which make it unworkable but I don't think those constraints are needed.
>>> You're starting from the position that the DT needs to be updated without
>>> the bootloader
>>
>> No, what I'm saying is that what the simplefb driver expects and what
>> the bootloader sets up may diverge as resource drivers are added to the
>> kernel. The DT /could/ be updated without the bootloader. You may only
>> be able to replace the DTB but not the bootloader on a given platform.
>
> simplefb should be a boot console and not survive past the boot
> process. Trying to get a 'generic' console driver to survive the boot
> process is not a generic problem. There are about 1,000 messages in
> these threads explaining why this is not a generic problem.
>
> All of these clock and regulator issues would go away by building a
> device specific framebuffer driver. A device specific framebuffer
> driver can be written in a day or two, it is far simpler than a KMS
> driver. This driver would how to parse the device specific device tree
> node and do the right thing with the regulators/clocks.
How it would know?
You need different clocks for LCD and different clocks for HDMI.
Unless it is a real driver that can drive either it can tell which is
enabled or u-boot has to tell it or you have to write a fixed entry
for the configuration you want in the DT and configure u-boot
accordingly by hand as well.
>
> So simplefb is built-in and used for early boot. During the boot
> process a device specific framebuffer driver loads. This device
> specific driver takes over for simplefb and can become the user space
> console.
>
> If the device specific framebuffer does not get loaded, then simplefb
> is going to stop working when the clocks and regulators get shut off.
> But that is what should happen.
Why it should be so?
It is reasonable to want working console on device which has u-boot or
other firmware graphics support but the support in kernel is still
under development.
Also the 'boot end' for kernel when it frees the clocks is way earlier
than the 'boot end' for the distribution which ends when you reach
certain init goal like multiuser environment. There is a lot between
and once the kernel hands over to init it can never tell what's going
on.
Since a modular KMS driver would load way later than the moment when
'boot end' is reached for kernel the simple function as early console
would break.
Also if you prevented resource management from happening during this
'booting' stage you could not safely load drivers during that time
which kind of defeats the purpose of this stage.
Because either the kernel can do resource management and give
resources to drivers that are loaded or it cannot do either.
Thanks
Michal
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-01 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMqctToYTw=rTOrjA__s6TdivBua0yvK3+uRzfcdTOxQjcTAg@mail.gmail.com>
On Wed, Oct 1, 2014 at 9:17 AM, Michal Suchanek <hramrach@gmail.com> wrote:
> On 1 October 2014 15:01, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
>>> On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote:
>>>> On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote:
>>>> > On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote:
>>>>
>>>> > > You may well need to extend the binding in future for an actual driver
>>>> > > but from the point of view of what's going into the block it really
>>>> > > should just be a case of reading the datasheet and mechanically typing
>>>> > > that in. If we can work out how to say where the framebuffer is we
>>>> > > really ought to be able to work this stuff out.
>>>>
>>>> > I agree from a technical point of view. However given the dynamically
>>>> > generated nature of the node the problem is more of a logistical nature.
>>>> > As we've seen U-Boot is being enabled to add clocks to the simplefb node
>>>> > but I'm fairly certain that there's a regulator somewhere that needs to
>>>> > be enabled too, be it for powering the display controller, the panel or
>>>> > the backlight. I wouldn't even be surprised if there were one for each
>>>> > of those. If so simplefb on this board will break when the regulators
>>>> > are described in the kernel's DTS.
>>>>
>>>> > If we don't consider this a problem then the whole DT ABI stability
>>>> > business is a farce.
>>>>
>>>> I think you're setting constraints on the implementation you want to see
>>>> which make it unworkable but I don't think those constraints are needed.
>>>> You're starting from the position that the DT needs to be updated without
>>>> the bootloader
>>>
>>> No, what I'm saying is that what the simplefb driver expects and what
>>> the bootloader sets up may diverge as resource drivers are added to the
>>> kernel. The DT /could/ be updated without the bootloader. You may only
>>> be able to replace the DTB but not the bootloader on a given platform.
>>
>> simplefb should be a boot console and not survive past the boot
>> process. Trying to get a 'generic' console driver to survive the boot
>> process is not a generic problem. There are about 1,000 messages in
>> these threads explaining why this is not a generic problem.
>>
>> All of these clock and regulator issues would go away by building a
>> device specific framebuffer driver. A device specific framebuffer
>> driver can be written in a day or two, it is far simpler than a KMS
>> driver. This driver would how to parse the device specific device tree
>> node and do the right thing with the regulators/clocks.
>
> How it would know?
>
> You need different clocks for LCD and different clocks for HDMI.
>
> Unless it is a real driver that can drive either it can tell which is
> enabled or u-boot has to tell it or you have to write a fixed entry
> for the configuration you want in the DT and configure u-boot
> accordingly by hand as well.
Start building all of that very device specific support inside the
device specific framebuffer driver. The device specific framebuffer
driver will be on initrd and it will get loaded as soon as possible by
the kernel.
Inside the device node for the video device there should be a sub-node
or phandle indicating the presence of the LCD or HDMI jack. That is a
valid hardware description and it should always be there. You can
also add a 'chosen' node to indicate how these have been programmed.
----------------------------------------
Two solutions --
1) Build in all of the device specific KMS/framebuffer drivers into
the kernel. Now there is no need for simplefb. But.... that wastes a
lot of memory with code that will never get executed.
2) Early boot off from simplefb. Have all of the graphics drivers on
initrd. Load the right one. Device specific graphic driver now owns
hardware. When KMS is missing, write a much smaller framebuffer
driver. You can start by copying simplefb and then add in the device
specific bits.
The option of fully booting on simplefb up to user space console is
not a good one. It requires that simplefb be taught about all of the
crazy and very complex clock and regulator environments for all of the
random graphics systems. And we're just getting started on enumerating
all of those crazy configurations. You haven't wandered into the area
of the video hardware living on a different bus and having a bus
controller in the way yet. Now you have to figure out how to keep that
bus from being turned off (there are SGI systems like this).
>
>>
>> So simplefb is built-in and used for early boot. During the boot
>> process a device specific framebuffer driver loads. This device
>> specific driver takes over for simplefb and can become the user space
>> console.
>>
>> If the device specific framebuffer does not get loaded, then simplefb
>> is going to stop working when the clocks and regulators get shut off.
>> But that is what should happen.
>
> Why it should be so?
>
> It is reasonable to want working console on device which has u-boot or
> other firmware graphics support but the support in kernel is still
> under development.
For the last 30 years this has been handled by using a UART console.
It is almost impossible to develop a framebuffer console driver while
also using the framebuffer for your UI.
>
> Also the 'boot end' for kernel when it frees the clocks is way earlier
> than the 'boot end' for the distribution which ends when you reach
simplefb just has to last until the kernel can modprobe in the device
specific framebuffer driver from initrd.
> certain init goal like multiuser environment. There is a lot between
> and once the kernel hands over to init it can never tell what's going
> on.
>
> Since a modular KMS driver would load way later than the moment when
> 'boot end' is reached for kernel the simple function as early console
> would break.
Whenever KMS gets written the base piece of it should going into
initrd and replace the device specific framebuffer driver. The device
specific framebuffer driver is just a filler until the real KMS driver
gets written. It needs to be a good filler since it may be years until
the KMS driver shows up.
>
> Also if you prevented resource management from happening during this
> 'booting' stage you could not safely load drivers during that time
> which kind of defeats the purpose of this stage.
>
> Because either the kernel can do resource management and give
> resources to drivers that are loaded or it cannot do either.
>
> Thanks
>
> Michal
>
> --
> 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@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-10-01 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001124800.GA21733@ulmo>
[-- Attachment #1: Type: text/plain, Size: 658 bytes --]
On Wed, Oct 01, 2014 at 02:48:02PM +0200, Thierry Reding wrote:
> On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote:
> > So don't do that if you're worried about it then, provide the bits of DT
> > that hook everything up from the start or otherwise describe things as
> > being in use.
> "Otherwise describe things as being in use" doesn't work for clocks for
> example. And Mike already said he wasn't willing to add something like
> an always-on DT property for clocks.
That's not the only way of doing things - another way would be to have a
stub driver that just holds the resources while working on getting a
full one in place for example.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-10-01 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001124852.GB21733@ulmo>
[-- Attachment #1: Type: text/plain, Size: 2219 bytes --]
On Wed, Oct 01, 2014 at 02:48:53PM +0200, Thierry Reding wrote:
> On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote:
> > I think you're setting constraints on the implementation you want to see
> > which make it unworkable but I don't think those constraints are needed.
> > You're starting from the position that the DT needs to be updated without
> > the bootloader
> No, what I'm saying is that what the simplefb driver expects and what
> the bootloader sets up may diverge as resource drivers are added to the
> kernel. The DT /could/ be updated without the bootloader. You may only
> be able to replace the DTB but not the bootloader on a given platform.
Sure, but doing that and also having the bootloader write part of the DT
from scratch with no cooperation from the rest of the DT doesn't seem
like the way to robustness.
> > and that the DT must not contain any hint of simplefb as
> > shipped separately.
> Well, I don't think it should because it describes the same resources
> that the device tree node for the real device already describes. But
> perhaps this is one of the cases where duplication isn't all that bad?
If we were worried about this wecould also do it by referring to
those nodes and saying "get all the resources these things need" rather
than duplicating the references (this might make it easier to work out
when the system is ready to hand off to the real drivers).
> > That's never going to work well as far as I can see
> > but doesn't seem like an ABI stability issue, or at least not a
> > reasonable one.
> It would work well under the assumption that the kernel wouldn't be
> touching any of the resources that simplefb depends on. If that's not a
> reasonable assumption then I think we can't make simplefb work the way
> it's currently written.
I can't see how that's reasonable unless the kernel has some way of
figuring out what it shouldn't be touching.
> > Either the bootloader needs to be updated along with the DT
> I thought we had decided that this was one of the big no-nos. But
> perhaps I'm misremembering.
It makes things more fragile so it's not desirable, no.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-01 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141001170517.GF4273@sirena.org.uk>
Hi,
On 10/01/2014 07:05 PM, Mark Brown wrote:
> On Wed, Oct 01, 2014 at 02:48:02PM +0200, Thierry Reding wrote:
>> On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote:
>
>>> So don't do that if you're worried about it then, provide the bits of DT
>>> that hook everything up from the start or otherwise describe things as
>>> being in use.
>
>> "Otherwise describe things as being in use" doesn't work for clocks for
>> example. And Mike already said he wasn't willing to add something like
>> an always-on DT property for clocks.
>
> That's not the only way of doing things - another way would be to have a
> stub driver that just holds the resources while working on getting a
> full one in place for example.
That won't work because the real driver which will eventually replace the
stub one will likely be a module, and then we will loose video output
between the kernel finalizing the initial boot, and the module actually
loading.
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.
This madness has to end! Thierry can we please have a clear and
unambiguous NACK from you on having the clocks property in the simplefb
dt node, and if you do so I expect a proof of concept patch from you
with an alternative solution within a week, or can you please stop
blocking this from getting merged?
And again, if you believe this will cause some sort of dt compat
issues or whatever, no one is making you use this property for
your boards!
Regards,
Hans
^ 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