From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@free-electrons.com (Maxime Ripard) Date: Wed, 13 Aug 2014 19:01:06 +0200 Subject: [PATCH 4/4] simplefb: add clock handling code In-Reply-To: <53EB9471.3090204@wwwdotorg.org> References: <1407914239-12054-1-git-send-email-libv@skynet.be> <1407914239-12054-5-git-send-email-libv@skynet.be> <53EB9471.3090204@wwwdotorg.org> Message-ID: <20140813170106.GT15297@lukather> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote: > On 08/13/2014 01:17 AM, Luc Verhaegen wrote: > >This claims and enables clocks listed in the simple framebuffer dt node. > >This is needed so that the display engine, in case the required clocks > >are known by the kernel code and are described in the dt, will remain > >properly enabled. > > I think this make simplefb not simple any more, which rather goes > against the whole point of it. > > I specifically conceived simplefb to know about nothing more than > the memory address and pixel layout of the memory buffer. I > certainly don't like the idea of expanding simplefb to anything > beyond that, and IIRC *not* extending is was a condition agreed when > it was first merged. If more knowledge than that is required, then > there needs to be a HW-specific driver to manage any > clocks/resets/video registers, etc. I'm sorry, but how is that not simple? clocks enabling is step 1 in a driver in order to communicate somehow with the controller. Reset is a different story, because arguably, if simplefb is there, the controller is already out of reset. And I don't see why video registers are coming into the discussion here. The code Luc posted doesn't access any register, at all. It just makes sure the needed controller keep going. > The correct way to handle this without a complete DRM/KMS/... driver > is to avoid the clocks in question being turned off at boot. Which is exactly what this code does, using the generic DT bindings to express dependency for a given clock. How is this wrong? > I thought there was a per-clock flag to prevent disabling an unused > clock? No, last time I heard, Mike Turquette was against it. > If not, perhaps the clock driver should force the clock to be > enabled (perhaps only if the DRM/KMS driver isn't enabled?). I'm sorry, but I'm not going to take any code that will do that in our clock driver. I'm not going to have a huge list of ifdef depending on configuration options to know which clock to enable, especially when clk_get should have the consumer device as an argument. > For example, the Tegra clock driver has a clock initialization table > which IIRC was used for this purpose before we got a DRM/KMS driver. > That way, all the details are kept inside the kernel code, and don't > end up influencing the DT representation of simplefb. I don't really see how the optional usage of a generic property influences badly the DT representation of simplefb. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: