Message-ID: <CAGb2v674q4Cjp9qy9CbUXe2j5pT_ez98CKxXHC8u4Diap2TT1A@mail.gmail.com>
Date:	Sat, 10 Jan 2015 00:29:32 +0800
From:	Chen-Yu Tsai <wens@...e.org>
To:	Gregory CLEMENT <gregory.clement@...e-electrons.com>
Cc:	Hans de Goede <hdegoede@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Boris BREZILLON <boris.brezillon@...e-electrons.com>,
	Jason Cooper <jason@...edaemon.net>,
	Tawfik Bayouk <tawfik@...vell.com>,
	Andrew Lunn <andrew@...n.ch>,
	devicetree <devicetree@...r.kernel.org>,
	Antoine Ténart <antoine.tenart@...e-electrons.com>,
	Mark Rutland <mark.rutland@....com>,
	Nadav Haklai <nadavh@...vell.com>, linux-ide@...r.kernel.org,
	Lior Amsalem <alior@...vell.com>,
	Mark Brown <broonie@...nel.org>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Tejun Heo <tj@...nel.org>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH v2 2/4] Documentation: bindings: Add the regulator
 property to the sub-nodes AHCI bindings

Hi,

On Sat, Jan 10, 2015 at 12:20 AM, Gregory CLEMENT
<gregory.clement@...e-electrons.com> wrote:
> Hi Hans,
>
> On 09/01/2015 16:46, Hans de Goede wrote:
>> Hi,
>>
>> On 09-01-15 11:39, Gregory CLEMENT wrote:
>>> It is now possible to use a regulator property for each port of the
>>> AHCI controller.
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@...e-electrons.com>
>>> ---
>>>   Documentation/devicetree/bindings/ata/ahci-platform.txt | 9 ++++++---
>>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>>> index 4ab09f2202d4..87416d71c758 100644
>>> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
>>> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>>> @@ -37,9 +37,10 @@ Required properties when using sub-nodes:
>>>
>>>
>>>   Sub-nodes required properties:
>>> -- reg               : the port number
>>> -- phys              : reference to the SATA PHY node
>>> -
>>> +- reg                                       : the port number
>>> +And at least one of the following properties:
>>> +- phys                      : reference to the SATA PHY node
>>> +- target-port<n>-supply     : regulator for SATA target power for port <n>
>>>
>>>   Examples:
>>>           sata@...08000 {
>>> @@ -68,10 +69,12 @@ With sub-nodes:
>>>              sata0: sata-port@0 {
>>>                      reg = <0>;
>>>                      phys = <&sata_phy 0>;
>>> +                    target-port0-supply = <&reg_sata0>;
>>>              };
>>>
>>
>> I'm sorry, but I've to NAK this, I did not follow the regulator discussion.,
>> thinking that it was just about the use of some utility function vs diy or
>> some such, I had no idea this would impact the dt-binding.
>>
>> The -port%d- bit is completely unnecessary from a dt pov. Devicetree is
>> supposed to be OS agnostic, we cannot make the dt-binding ugly just because
>> the regulator subsys in Linux does not provide the necessary functionality.
>>
>> I'm afraid you will have to get back to the regulator subsys people and tell
>> them that we really need an of_regulator_get, so that we can tell the
>> regulator subsys we want a regulator from a specific of_node.
>
> I didn't cc you especially, but I cc the linux-ide mailing list. The
> email was "[PATCH 2/2] regulator: core: Add the device tree version to
> the regulator_get family" (see
> http://thread.gmane.org/gmane.linux.kernel/1856154).
>
> Mark Brown didn't want a firmware specific interface. The regulator
> framework allows associating only a regulator to a device and
> currently, at least from the point of view of the device tree, the
> port subnodes are not devices. He suggested making the child node
> device. However, when I had a look on it, it didn't seem an easy
> task. As you know better this code, what do you think of it?
>
> (I added Mark Bronw in CC of this email so he can add more explanation
> if needed)

Since the AHCI library code already supports the generic phy subsystem,
with one phy possible for each port node, you could possibly add a
generic phy that just takes a regulator, and hook it up that way.

I don't know if the extra layer of indirection is good or not.
Just offering an alternative.


Regards,
ChenYu

>>
>> Please CC me when you re-open the discussion with the regulator people.
>>
>> Regards,
>>
>> Hans
>>
>
>
> --
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists