From: Mark Brown <broonie@kernel.org> To: Hans de Goede <hdegoede@redhat.com> Cc: "Chen-Yu Tsai" <wens@csie.org>, "Gregory CLEMENT" <gregory.clement@free-electrons.com>, linux-kernel <linux-kernel@vger.kernel.org>, "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Boris BREZILLON" <boris.brezillon@free-electrons.com>, "Jason Cooper" <jason@lakedaemon.net>, "Tawfik Bayouk" <tawfik@marvell.com>, "Andrew Lunn" <andrew@lunn.ch>, devicetree <devicetree@vger.kernel.org>, "Antoine Ténart" <antoine.tenart@free-electrons.com>, "Mark Rutland" <mark.rutland@arm.com>, "Nadav Haklai" <nadavh@marvell.com>, linux-ide@vger.kernel.org, "Lior Amsalem" <alior@marvell.com>, "Ezequiel Garcia" <ezequiel.garcia@free-electrons.com>, "Tejun Heo" <tj@kernel.org>, "Maxime Ripard" <maxime.ripard@free-electrons.com>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, "Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>, "Ulf Hansson" <ulf.hansson@linaro.org> Subject: Re: [PATCH v2 2/4] Documentation: bindings: Add the regulator property to the sub-nodes AHCI bindings Date: Sat, 10 Jan 2015 11:17:48 +0000 [thread overview] Message-ID: <20150110111748.GB4160@sirena.org.uk> (raw) In-Reply-To: <54B0FCDD.7000300@redhat.com> [-- Attachment #1: Type: text/plain, Size: 2668 bytes --] On Sat, Jan 10, 2015 at 11:20:13AM +0100, Hans de Goede wrote: > On 09-01-15 18:11, Mark Brown wrote: > >Or if the supply is for the device at the other end of the link (which > >is what it sounded like) then use that. This just sounds like the same > >problem we have for all the enumerable buses in embedded systems where > >we need to be able to understand that the device exists prior to it > >being fully ready to appear in the system. Having the link/slot be a > >device in Linux does indeed seem to be a common way people think about > >doing this, it sounds like for this one it might be the most direct. > I think we should be careful to not think too much from an implementation > pov here, the purpose of the devicetree description is to describe the hardware, > as is. I don't think anyone is talking about changing the DT here, only the way it's represented inside Linux. > sata0: sata-port@0 { > reg = <0>; > phys = <&sata_phy 0>; > target-supply = <®_sata0>; > }; > > sata1: sata-port@1 { > reg = <1>; > phys = <&sata_phy 1>; > target-supply = <®_sata1>; > }; > }; > Seems to match the hardware pretty exactly, and also matches how we've > been describing similar devices with only one sata port + power plug sofar, > so from a consistency pov it also is a good model. Right, I think that makes sense - it also looks to me like these things should be representable as devices within Linux. > So supporting this model requires having a regulator_get API which allows > specifying which of_node to get the regulator from, e.g. the proposed It requires nothing of the sort. > of_regulator_get function. I know you (Mark) do not like this, but all > other subsystems have an of_foo_get function taking an of_node as argument, > I do not see how the regulator subsys is so special that it cannot have one, > and also notice that this is only a kernel internal API which we can always > change later. Two things there. One is that mostly those APIs are legacy APIs from before we made DT a first class citizen and generally they shouldn't be used these days. The other is that at least for regulators we have constant problems with people abusing the API in various ways, as a result the API has a goal of not helping undesirable usage patterns in order to help people spot problems. Having an API which makes it easy create broken bindings means that it is much more likely that people will do just that. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: broonie@kernel.org (Mark Brown) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 2/4] Documentation: bindings: Add the regulator property to the sub-nodes AHCI bindings Date: Sat, 10 Jan 2015 11:17:48 +0000 [thread overview] Message-ID: <20150110111748.GB4160@sirena.org.uk> (raw) In-Reply-To: <54B0FCDD.7000300@redhat.com> On Sat, Jan 10, 2015 at 11:20:13AM +0100, Hans de Goede wrote: > On 09-01-15 18:11, Mark Brown wrote: > >Or if the supply is for the device at the other end of the link (which > >is what it sounded like) then use that. This just sounds like the same > >problem we have for all the enumerable buses in embedded systems where > >we need to be able to understand that the device exists prior to it > >being fully ready to appear in the system. Having the link/slot be a > >device in Linux does indeed seem to be a common way people think about > >doing this, it sounds like for this one it might be the most direct. > I think we should be careful to not think too much from an implementation > pov here, the purpose of the devicetree description is to describe the hardware, > as is. I don't think anyone is talking about changing the DT here, only the way it's represented inside Linux. > sata0: sata-port at 0 { > reg = <0>; > phys = <&sata_phy 0>; > target-supply = <®_sata0>; > }; > > sata1: sata-port at 1 { > reg = <1>; > phys = <&sata_phy 1>; > target-supply = <®_sata1>; > }; > }; > Seems to match the hardware pretty exactly, and also matches how we've > been describing similar devices with only one sata port + power plug sofar, > so from a consistency pov it also is a good model. Right, I think that makes sense - it also looks to me like these things should be representable as devices within Linux. > So supporting this model requires having a regulator_get API which allows > specifying which of_node to get the regulator from, e.g. the proposed It requires nothing of the sort. > of_regulator_get function. I know you (Mark) do not like this, but all > other subsystems have an of_foo_get function taking an of_node as argument, > I do not see how the regulator subsys is so special that it cannot have one, > and also notice that this is only a kernel internal API which we can always > change later. Two things there. One is that mostly those APIs are legacy APIs from before we made DT a first class citizen and generally they shouldn't be used these days. The other is that at least for regulators we have constant problems with people abusing the API in various ways, as a result the API has a goal of not helping undesirable usage patterns in order to help people spot problems. Having an API which makes it easy create broken bindings means that it is much more likely that people will do just that. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150110/70c1fc6a/attachment.sig>
next prev parent reply other threads:[~2015-01-10 11:17 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-09 10:39 [PATCH v2 0/4] ata: libahci: Allow using a regulator for each port Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` [PATCH v2 1/4] ata: libahci: Clean-up the ahci_platform_en/disable_phys functions Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` [PATCH v2 3/4] ata: libahci: Allow using multiple regulators Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT [not found] ` <1420799989-10645-1-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 2015-01-09 10:39 ` [PATCH v2 2/4] Documentation: bindings: Add the regulator property to the sub-nodes AHCI bindings Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 15:46 ` Hans de Goede 2015-01-09 15:46 ` Hans de Goede 2015-01-09 16:20 ` Gregory CLEMENT 2015-01-09 16:20 ` Gregory CLEMENT 2015-01-09 16:29 ` Chen-Yu Tsai 2015-01-09 16:29 ` Chen-Yu Tsai 2015-01-09 17:11 ` Mark Brown 2015-01-09 17:11 ` Mark Brown [not found] ` <20150109171131.GH2634-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2015-01-10 10:20 ` Hans de Goede 2015-01-10 10:20 ` Hans de Goede 2015-01-10 10:20 ` Hans de Goede 2015-01-10 11:17 ` Mark Brown [this message] 2015-01-10 11:17 ` Mark Brown 2015-01-10 13:51 ` Hans de Goede 2015-01-10 13:51 ` Hans de Goede 2015-01-12 12:49 ` Gregory CLEMENT 2015-01-12 12:49 ` Gregory CLEMENT 2015-01-09 10:39 ` [PATCH v2 4/4] ARM: mvebu: Armada 385 GP: Add regulators to the SATA port Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT 2015-01-09 10:39 ` Gregory CLEMENT
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20150110111748.GB4160@sirena.org.uk \ --to=broonie@kernel.org \ --cc=alior@marvell.com \ --cc=andrew@lunn.ch \ --cc=antoine.tenart@free-electrons.com \ --cc=boris.brezillon@free-electrons.com \ --cc=devicetree@vger.kernel.org \ --cc=ezequiel.garcia@free-electrons.com \ --cc=gregory.clement@free-electrons.com \ --cc=hdegoede@redhat.com \ --cc=jason@lakedaemon.net \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-ide@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=maxime.ripard@free-electrons.com \ --cc=nadavh@marvell.com \ --cc=sebastian.hesselbarth@gmail.com \ --cc=tawfik@marvell.com \ --cc=thomas.petazzoni@free-electrons.com \ --cc=tj@kernel.org \ --cc=ulf.hansson@linaro.org \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.