From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Brown Subject: Re: Should regulator core support parsing OF based fwnode? Date: Fri, 4 Oct 2019 15:40:29 +0100 Message-ID: <20191004144029.GC4866@sirena.co.uk> References: <20191003082812.28491-3-jjhiblot@ti.com> <20191003104228.c5nho6eimwzqwxpt@earth.universe> <62591735-9082-1fd7-d791-07929ddaa223@gmail.com> <20191003183554.GA37096@sirena.co.uk> <25b9614f-d6be-9da5-0fe5-eb58c8c93850@gmail.com> <20191003194140.GE6090@sirena.co.uk> <20191004113942.GB4866@sirena.co.uk> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1290676362==" Return-path: Received: from heliosphere.sirena.org.uk (heliosphere.sirena.org.uk [172.104.155.198]) by gabe.freedesktop.org (Postfix) with ESMTPS id C92AE6EBB9 for ; Fri, 4 Oct 2019 14:40:33 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jean-Jacques Hiblot Cc: mark.rutland@arm.com, daniel.thompson@linaro.org, Liam Girdwood , tomi.valkeinen@ti.com, Sebastian Reichel , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, Jacek Anaszewski , pavel@ucw.cz, lee.jones@linaro.org, linux-leds@vger.kernel.org, dmurphy@ti.com List-Id: dri-devel@lists.freedesktop.org --===============1290676362== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="V88s5gaDVPzZ0KCq" Content-Disposition: inline --V88s5gaDVPzZ0KCq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 04, 2019 at 03:33:13PM +0200, Jean-Jacques Hiblot wrote: > On 04/10/2019 13:39, Mark Brown wrote: > > Consumers should just be able to request a regulator without having to > > worry about how that's being provided - they should have no knowledge at > > all of firmware bindings or platform data for defining this. If they > > do that suggests there's an abstraction issue somewhere, what makes you > > think that doing something with of_node is required? > The regulator core accesses consumer->of_node to get a phandle to a > regulator's node. The trouble arises from the fact that the LED core does > not populate of_node anymore, instead it populates fwnode. This allows the > LED core to be agnostic of ACPI or OF to get the properties of a LED. Why is the LED core populating anything? Is the LED core copying bits out of the struct device for the actual device into a synthetic device rather than passing the actual device in? That really doesn't seem like a good idea, it's likely to lead to things like this where you don't copy something that's required (or worse where something directly in the struct device that can't be copied is needed). > IMO it is better to populate both of_node and fwnode in the LED core at the > moment. It has already been fixed this way for the platform driver [0], MTD > [1] and PCI-OF [2]. Yeah, if you're going to be copying stuff out of the real device I'd copy the of_node as well. > > Further, unless you have LEDs that work without power you probably > > shouldn't be using _get_optional() for their supply. That interface is > > intended only for supplies that may be physically absent. > Not all LEDs have a regulator to provide the power. The power can be > supplied by the LED controller for example. This code probably shouldn't be being run at all for LEDs like that, I was assuming this was just for GPIO LEDs and similar rather than all LEDs. --V88s5gaDVPzZ0KCq Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl2XWdwACgkQJNaLcl1U h9A0xwf/U7Fm4ePiL2y1R3tDNXQn6f+Ur82WCmb5QG5/fkiK7tKAiiqjZdb4UiQ3 UoCAY3wUBeivTw0fCmNqy4kzoR8YY+pOlVu5Tp1j2XMTO/xZTTfnW6oYBJai4evk 6rUCxFl/STljjjojTz+dph2hqLkBwo1HbPKqwXXHvy54CIalHfRma0AbN4t1qySt WftXW83OaF82QqZLtf21nnKfAuve/p8nPEc5wjF06ZFLqv2LLRBEwJWXc2L3O0NC FgOKXCWKQwidU2XcgS67Kxtz0vviOhugRZm9boLYk/63sMjOnYmS6+9OzzSgFXCI Q+eZGtEugXuSLTew99ZJm9Hw3FTj7Q== =SmQ4 -----END PGP SIGNATURE----- --V88s5gaDVPzZ0KCq-- --===============1290676362== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============1290676362==--