[v3,1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
diff mbox

Message ID 1465204804-31161-2-git-send-email-frank.wang@rock-chips.com
State New
Headers show

Commit Message

Frank Wang June 6, 2016, 9:20 a.m. UTC
Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
---

Changes in v3:
 - Added 'clocks' and 'clock-names' optional properties.
 - Specified 'otg-port' and 'host-port' as the sub-node name.

Changes in v2:
 - Changed vbus_host optional property from gpio to regulator.
 - Specified vbus_otg-supply optional property.
 - Specified otg_id and otg_bvalid property.

 .../bindings/phy/phy-rockchip-inno-usb2.txt        |   60 ++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt

Comments

Mark Rutland June 6, 2016, 11:27 a.m. UTC | #1
On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> ---
> 
> Changes in v3:
>  - Added 'clocks' and 'clock-names' optional properties.
>  - Specified 'otg-port' and 'host-port' as the sub-node name.
> 
> Changes in v2:
>  - Changed vbus_host optional property from gpio to regulator.
>  - Specified vbus_otg-supply optional property.
>  - Specified otg_id and otg_bvalid property.
> 
>  .../bindings/phy/phy-rockchip-inno-usb2.txt        |   60 ++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> new file mode 100644
> index 0000000..0b4bbbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -0,0 +1,60 @@
> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> +
> +Required properties (phy (parent) node):
> + - compatible : should be one of the listed compatibles:
> +	* "rockchip,rk3366-usb2phy"
> +	* "rockchip,rk3399-usb2phy"
> + - #clock-cells : should be 0.
> + - clock-output-names : specify the 480m output clock name.
> +
> +Optional properties:
> + - clocks : phandle + phy specifier pair, for the input clock of phy.
> + - clock-names : input clock name of phy, must be "phyclk".
> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.

Nit: s/_/-/ here.

Otherwise the rest of this looks generally fine, though I'm confused as
to how you address the programming interface(s), given none are
described.

Thanks,
Mark.

> +
> +Required nodes : a sub-node is required for each port the phy provides.
> +		 The sub-node name is used to identify host or otg port,
> +		 and shall be the following entries:
> +	* "otg-port" : the name of otg port.
> +	* "host-port" : the name of host port.
> +
> +Required properties (port (child) node):
> + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
> + - interrupts : specify an interrupt for each entry in interrupt-names.
> + - interrupt-names : a list which shall be the following entries:
> +	* "otg_id" : for the otg id interrupt.
> +	* "otg_bvalid" : for the otg vbus interrupt.
> +	* "linestate" : for the host/otg linestate interrupt.
> +
> +Example:
> +
> +grf: syscon@ff770000 {
> +	compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +...
> +
> +	u2phy: usb2-phy {
> +		compatible = "rockchip,rk3366-usb2phy";
> +		#clock-cells = <0>;
> +		clock-output-names = "sclk_otgphy0_480m";
> +
> +		u2phy_otg: otg-port {
> +			#phy-cells = <0>;
> +			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "otg_id", "otg_bvalid", "linestate";
> +			status = "okay";
> +		};
> +
> +		u2phy_host: host-port {
> +			#phy-cells = <0>;
> +			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "linestate";
> +			status = "okay";
> +		};
> +	};
> +};
> -- 
> 1.7.9.5
> 
>
Heiko Stübner June 6, 2016, 12:33 p.m. UTC | #2
Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
> > Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> > ---
> > 
> > Changes in v3:
> >  - Added 'clocks' and 'clock-names' optional properties.
> >  - Specified 'otg-port' and 'host-port' as the sub-node name.
> > 
> > Changes in v2:
> >  - Changed vbus_host optional property from gpio to regulator.
> >  - Specified vbus_otg-supply optional property.
> >  - Specified otg_id and otg_bvalid property.
> >  
> >  .../bindings/phy/phy-rockchip-inno-usb2.txt        |   60
> >  ++++++++++++++++++++ 1 file changed, 60 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
> > file mode 100644
> > index 0000000..0b4bbbb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > @@ -0,0 +1,60 @@
> > +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> > +
> > +Required properties (phy (parent) node):
> > + - compatible : should be one of the listed compatibles:
> > +	* "rockchip,rk3366-usb2phy"
> > +	* "rockchip,rk3399-usb2phy"
> > + - #clock-cells : should be 0.
> > + - clock-output-names : specify the 480m output clock name.
> > +
> > +Optional properties:
> > + - clocks : phandle + phy specifier pair, for the input clock of phy.
> > + - clock-names : input clock name of phy, must be "phyclk".
> > + - vbus_host-supply : phandle to a regulator that supplies host vbus.
> > + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
> 
> Nit: s/_/-/ here.

Something I only stumbled over yesterday for the first time on my rk3288-
popmetal: The phy subnodes seem to be able to use a generic phy-supply
property from inside the phy-core itself, see:

https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597

for my WIP code for that other board.


> Otherwise the rest of this looks generally fine, though I'm confused as
> to how you address the programming interface(s), given none are
> described.

I think that comes generally down to phy_power_on and phy_power_off from the
host driver (ehci / dwc2 / whatever) using the generic phy interface. The usb2
phys on Rockchip SoCs seem always pretty easy to handle, while the new
additional typeC phy seems to require more work


Heiko

> > +
> > +Required nodes : a sub-node is required for each port the phy provides.
> > +		 The sub-node name is used to identify host or otg port,
> > +		 and shall be the following entries:
> > +	* "otg-port" : the name of otg port.
> > +	* "host-port" : the name of host port.
> > +
> > +Required properties (port (child) node):
> > + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
> > + - interrupts : specify an interrupt for each entry in interrupt-names.
> > + - interrupt-names : a list which shall be the following entries:
> > +	* "otg_id" : for the otg id interrupt.
> > +	* "otg_bvalid" : for the otg vbus interrupt.
> > +	* "linestate" : for the host/otg linestate interrupt.
> > +
> > +Example:
> > +
> > +grf: syscon@ff770000 {
> > +	compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +
> > +...
> > +
> > +	u2phy: usb2-phy {
> > +		compatible = "rockchip,rk3366-usb2phy";
> > +		#clock-cells = <0>;
> > +		clock-output-names = "sclk_otgphy0_480m";
> > +
> > +		u2phy_otg: otg-port {
> > +			#phy-cells = <0>;
> > +			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "otg_id", "otg_bvalid", "linestate";
> > +			status = "okay";
> > +		};
> > +
> > +		u2phy_host: host-port {
> > +			#phy-cells = <0>;
> > +			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "linestate";
> > +			status = "okay";
> > +		};
> > +	};
> > +};
Frank Wang June 7, 2016, 2:59 a.m. UTC | #3
Hi Heiko & Mark,

On 2016/6/6 20:33, Heiko Stübner wrote:
> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>> ---
>>>
>>> Changes in v3:
>>>   - Added 'clocks' and 'clock-names' optional properties.
>>>   - Specified 'otg-port' and 'host-port' as the sub-node name.
>>>
>>> Changes in v2:
>>>   - Changed vbus_host optional property from gpio to regulator.
>>>   - Specified vbus_otg-supply optional property.
>>>   - Specified otg_id and otg_bvalid property.
>>>   
>>>   .../bindings/phy/phy-rockchip-inno-usb2.txt        |   60
>>>   ++++++++++++++++++++ 1 file changed, 60 insertions(+)
>>>   create mode 100644
>>>   Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
>>> file mode 100644
>>> index 0000000..0b4bbbb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>> @@ -0,0 +1,60 @@
>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
>>> +
>>> +Required properties (phy (parent) node):
>>> + - compatible : should be one of the listed compatibles:
>>> +	* "rockchip,rk3366-usb2phy"
>>> +	* "rockchip,rk3399-usb2phy"
>>> + - #clock-cells : should be 0.
>>> + - clock-output-names : specify the 480m output clock name.
>>> +
>>> +Optional properties:
>>> + - clocks : phandle + phy specifier pair, for the input clock of phy.
>>> + - clock-names : input clock name of phy, must be "phyclk".
>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
>> Nit: s/_/-/ here.
> Something I only stumbled over yesterday for the first time on my rk3288-
> popmetal: The phy subnodes seem to be able to use a generic phy-supply
> property from inside the phy-core itself, see:
>
> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597
>
> for my WIP code for that other board.
>

Ah, good comments! I will try later, if it is practicable, I shall 
correct it into the next patches (patch v4).

>> Otherwise the rest of this looks generally fine, though I'm confused as
>> to how you address the programming interface(s), given none are
>> described.
> I think that comes generally down to phy_power_on and phy_power_off from the
> host driver (ehci / dwc2 / whatever) using the generic phy interface. The usb2
> phys on Rockchip SoCs seem always pretty easy to handle, while the new
> additional typeC phy seems to require more work
>

Yeah, just like that.

>
>
>>> +
>>> +Required nodes : a sub-node is required for each port the phy provides.
>>> +		 The sub-node name is used to identify host or otg port,
>>> +		 and shall be the following entries:
>>> +	* "otg-port" : the name of otg port.
>>> +	* "host-port" : the name of host port.
>>> +
>>> +Required properties (port (child) node):
>>> + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
>>> + - interrupts : specify an interrupt for each entry in interrupt-names.
>>> + - interrupt-names : a list which shall be the following entries:
>>> +	* "otg_id" : for the otg id interrupt.
>>> +	* "otg_bvalid" : for the otg vbus interrupt.
>>> +	* "linestate" : for the host/otg linestate interrupt.
>>> +
>>> +Example:
>>> +
>>> +grf: syscon@ff770000 {
>>> +	compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
>>> +	#address-cells = <1>;
>>> +	#size-cells = <1>;
>>> +
>>> +...
>>> +
>>> +	u2phy: usb2-phy {
>>> +		compatible = "rockchip,rk3366-usb2phy";
>>> +		#clock-cells = <0>;
>>> +		clock-output-names = "sclk_otgphy0_480m";
>>> +
>>> +		u2phy_otg: otg-port {
>>> +			#phy-cells = <0>;
>>> +			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>,
>>> +				     <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
>>> +				     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
>>> +			interrupt-names = "otg_id", "otg_bvalid", "linestate";
>>> +			status = "okay";
>>> +		};
>>> +
>>> +		u2phy_host: host-port {
>>> +			#phy-cells = <0>;
>>> +			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
>>> +			interrupt-names = "linestate";
>>> +			status = "okay";
>>> +		};
>>> +	};
>>> +};
Frank Wang June 7, 2016, 3:31 a.m. UTC | #4
Hi Heiko,

On 2016/6/7 10:59, Frank Wang wrote:
> Hi Heiko & Mark,
>
> On 2016/6/6 20:33, Heiko Stübner wrote:
>> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
>>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
>>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v3:
>>>>   - Added 'clocks' and 'clock-names' optional properties.
>>>>   - Specified 'otg-port' and 'host-port' as the sub-node name.
>>>>
>>>> Changes in v2:
>>>>   - Changed vbus_host optional property from gpio to regulator.
>>>>   - Specified vbus_otg-supply optional property.
>>>>   - Specified otg_id and otg_bvalid property.
>>>>     .../bindings/phy/phy-rockchip-inno-usb2.txt        | 60
>>>>   ++++++++++++++++++++ 1 file changed, 60 insertions(+)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
>>>> file mode 100644
>>>> index 0000000..0b4bbbb
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>> @@ -0,0 +1,60 @@
>>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
>>>> +
>>>> +Required properties (phy (parent) node):
>>>> + - compatible : should be one of the listed compatibles:
>>>> +    * "rockchip,rk3366-usb2phy"
>>>> +    * "rockchip,rk3399-usb2phy"
>>>> + - #clock-cells : should be 0.
>>>> + - clock-output-names : specify the 480m output clock name.
>>>> +
>>>> +Optional properties:
>>>> + - clocks : phandle + phy specifier pair, for the input clock of phy.
>>>> + - clock-names : input clock name of phy, must be "phyclk".
>>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
>>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
>>> Nit: s/_/-/ here.
>> Something I only stumbled over yesterday for the first time on my 
>> rk3288-
>> popmetal: The phy subnodes seem to be able to use a generic phy-supply
>> property from inside the phy-core itself, see:
>>
>> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597 
>>
>>
>> for my WIP code for that other board.
>>
>
> Ah, good comments! I will try later, if it is practicable, I shall 
> correct it into the next patches (patch v4).
>

I am sorry to tell you that seems unworkable, because we have two 
sub-nodes (phy-ports) in one parent-node (phy),
what is more, the 'phy-supply' property can only put into parent-node, I 
believe it can not be differentiated types of ports.
I mean vbus for host and otg are separately.

>>> Otherwise the rest of this looks generally fine, though I'm confused as
>>> to how you address the programming interface(s), given none are
>>> described.
>> I think that comes generally down to phy_power_on and phy_power_off 
>> from the
>> host driver (ehci / dwc2 / whatever) using the generic phy interface. 
>> The usb2
>> phys on Rockchip SoCs seem always pretty easy to handle, while the new
>> additional typeC phy seems to require more work
>>
>
> Yeah, just like that.
>
>>
>>
>>>> +
>>>> +Required nodes : a sub-node is required for each port the phy 
>>>> provides.
>>>> +         The sub-node name is used to identify host or otg port,
>>>> +         and shall be the following entries:
>>>> +    * "otg-port" : the name of otg port.
>>>> +    * "host-port" : the name of host port.
>>>> +
>>>> +Required properties (port (child) node):
>>>> + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
>>>> + - interrupts : specify an interrupt for each entry in 
>>>> interrupt-names.
>>>> + - interrupt-names : a list which shall be the following entries:
>>>> +    * "otg_id" : for the otg id interrupt.
>>>> +    * "otg_bvalid" : for the otg vbus interrupt.
>>>> +    * "linestate" : for the host/otg linestate interrupt.
>>>> +
>>>> +Example:
>>>> +
>>>> +grf: syscon@ff770000 {
>>>> +    compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
>>>> +    #address-cells = <1>;
>>>> +    #size-cells = <1>;
>>>> +
>>>> +...
>>>> +
>>>> +    u2phy: usb2-phy {
>>>> +        compatible = "rockchip,rk3366-usb2phy";
>>>> +        #clock-cells = <0>;
>>>> +        clock-output-names = "sclk_otgphy0_480m";
>>>> +
>>>> +        u2phy_otg: otg-port {
>>>> +            #phy-cells = <0>;
>>>> +            interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>,
>>>> +                     <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
>>>> +                     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
>>>> +            interrupt-names = "otg_id", "otg_bvalid", "linestate";
>>>> +            status = "okay";
>>>> +        };
>>>> +
>>>> +        u2phy_host: host-port {
>>>> +            #phy-cells = <0>;
>>>> +            interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
>>>> +            interrupt-names = "linestate";
>>>> +            status = "okay";
>>>> +        };
>>>> +    };
>>>> +};
Kishon Vijay Abraham I June 7, 2016, 7:45 a.m. UTC | #5
Hi,

On Tuesday 07 June 2016 09:01 AM, Frank Wang wrote:
> Hi Heiko,
> 
> On 2016/6/7 10:59, Frank Wang wrote:
>> Hi Heiko & Mark,
>>
>> On 2016/6/6 20:33, Heiko Stübner wrote:
>>> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
>>>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
>>>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>>   - Added 'clocks' and 'clock-names' optional properties.
>>>>>   - Specified 'otg-port' and 'host-port' as the sub-node name.
>>>>>
>>>>> Changes in v2:
>>>>>   - Changed vbus_host optional property from gpio to regulator.
>>>>>   - Specified vbus_otg-supply optional property.
>>>>>   - Specified otg_id and otg_bvalid property.
>>>>>     .../bindings/phy/phy-rockchip-inno-usb2.txt        | 60
>>>>>   ++++++++++++++++++++ 1 file changed, 60 insertions(+)
>>>>>   create mode 100644
>>>>> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
>>>>> file mode 100644
>>>>> index 0000000..0b4bbbb
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>>> @@ -0,0 +1,60 @@
>>>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
>>>>> +
>>>>> +Required properties (phy (parent) node):
>>>>> + - compatible : should be one of the listed compatibles:
>>>>> +    * "rockchip,rk3366-usb2phy"
>>>>> +    * "rockchip,rk3399-usb2phy"
>>>>> + - #clock-cells : should be 0.
>>>>> + - clock-output-names : specify the 480m output clock name.
>>>>> +
>>>>> +Optional properties:
>>>>> + - clocks : phandle + phy specifier pair, for the input clock of phy.
>>>>> + - clock-names : input clock name of phy, must be "phyclk".
>>>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
>>>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
>>>> Nit: s/_/-/ here.
>>> Something I only stumbled over yesterday for the first time on my rk3288-
>>> popmetal: The phy subnodes seem to be able to use a generic phy-supply
>>> property from inside the phy-core itself, see:
>>>
>>> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597
>>>
>>>
>>> for my WIP code for that other board.
>>>
>>
>> Ah, good comments! I will try later, if it is practicable, I shall correct it
>> into the next patches (patch v4).
>>
> 
> I am sorry to tell you that seems unworkable, because we have two sub-nodes
> (phy-ports) in one parent-node (phy),
> what is more, the 'phy-supply' property can only put into parent-node, I
> believe it can not be differentiated types of ports.

'phy-supply' is a property of the phy node and not the 'phy-provider' node. So
IMO this should work. What problem do you see?

Thanks
Kishon
Heiko Stübner June 7, 2016, 7:59 a.m. UTC | #6
Hi Frank,

Am Dienstag, 7. Juni 2016, 11:31:59 schrieb Frank Wang:
> On 2016/6/7 10:59, Frank Wang wrote:
> > On 2016/6/6 20:33, Heiko Stübner wrote:
> >> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
> >>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
> >>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> >>>> ---
> >>>> 
> >>>> Changes in v3:
> >>>>   - Added 'clocks' and 'clock-names' optional properties.
> >>>>   - Specified 'otg-port' and 'host-port' as the sub-node name.
> >>>> 
> >>>> Changes in v2:
> >>>>   - Changed vbus_host optional property from gpio to regulator.
> >>>>   - Specified vbus_otg-supply optional property.
> >>>>   - Specified otg_id and otg_bvalid property.
> >>>>   
> >>>>     .../bindings/phy/phy-rockchip-inno-usb2.txt        | 60
> >>>>   
> >>>>   ++++++++++++++++++++ 1 file changed, 60 insertions(+)
> >>>>   create mode 100644
> >>>> 
> >>>> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> >>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
> >>>> file mode 100644
> >>>> index 0000000..0b4bbbb
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> >>>> @@ -0,0 +1,60 @@
> >>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> >>>> +
> >>>> +Required properties (phy (parent) node):
> >>>> + - compatible : should be one of the listed compatibles:
> >>>> +    * "rockchip,rk3366-usb2phy"
> >>>> +    * "rockchip,rk3399-usb2phy"
> >>>> + - #clock-cells : should be 0.
> >>>> + - clock-output-names : specify the 480m output clock name.
> >>>> +
> >>>> +Optional properties:
> >>>> + - clocks : phandle + phy specifier pair, for the input clock of phy.
> >>>> + - clock-names : input clock name of phy, must be "phyclk".
> >>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
> >>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
> >>> 
> >>> Nit: s/_/-/ here.
> >> 
> >> Something I only stumbled over yesterday for the first time on my
> >> rk3288-
> >> popmetal: The phy subnodes seem to be able to use a generic phy-supply
> >> property from inside the phy-core itself, see:
> >> 
> >> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aa
> >> f6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597
> >> 
> >> 
> >> for my WIP code for that other board.
> > 
> > Ah, good comments! I will try later, if it is practicable, I shall
> > correct it into the next patches (patch v4).
> 
> I am sorry to tell you that seems unworkable, because we have two
> sub-nodes (phy-ports) in one parent-node (phy),
> what is more, the 'phy-supply' property can only put into parent-node, I
> believe it can not be differentiated types of ports.
> I mean vbus for host and otg are separately.

I would disagree ;-)

If you look in phy-core.c phy_create(), you can see that the struct phy that
gets created, contains its own struct device instance, which then gets the
phys of_node (the host+otg subnodes in this context) attached to it.

The regulator_get_optional then runs on this struct device, thus making lookup
on the subnode. And that works just nicely on my rk3288-popmetal, with its 3
phy subnodes and debugfs/regulator/regulator_summary prints that nicely:

 vcc_sys                          0   12      0  5000mV     0mA  5000mV  5000mV 
    vcc_host_5v                   1    1      0  5000mV     0mA  5000mV  5000mV 
       phy-phy.2                                                    0mV     0mV
    vcc_otg_5v                    1    1      0  5000mV     0mA  5000mV  5000mV 
       phy-phy.0                                                    0mV     0mV
    vcc_sata_5v                   2    1      0  5000mV     0mA  5000mV  5000mV 
       phy-phy.1                                                    0mV     0mV


Heiko
Frank Wang June 7, 2016, 8:23 a.m. UTC | #7
Hi Kishon & Heiko,

On 2016/6/7 15:45, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Tuesday 07 June 2016 09:01 AM, Frank Wang wrote:
>> Hi Heiko,
>>
>> On 2016/6/7 10:59, Frank Wang wrote:
>>> Hi Heiko & Mark,
>>>
>>> On 2016/6/6 20:33, Heiko Stübner wrote:
>>>> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
>>>>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
>>>>>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v3:
>>>>>>    - Added 'clocks' and 'clock-names' optional properties.
>>>>>>    - Specified 'otg-port' and 'host-port' as the sub-node name.
>>>>>>
>>>>>> Changes in v2:
>>>>>>    - Changed vbus_host optional property from gpio to regulator.
>>>>>>    - Specified vbus_otg-supply optional property.
>>>>>>    - Specified otg_id and otg_bvalid property.
>>>>>>      .../bindings/phy/phy-rockchip-inno-usb2.txt        | 60
>>>>>>    ++++++++++++++++++++ 1 file changed, 60 insertions(+)
>>>>>>    create mode 100644
>>>>>> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
>>>>>> file mode 100644
>>>>>> index 0000000..0b4bbbb
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>>>>>> @@ -0,0 +1,60 @@
>>>>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
>>>>>> +
>>>>>> +Required properties (phy (parent) node):
>>>>>> + - compatible : should be one of the listed compatibles:
>>>>>> +    * "rockchip,rk3366-usb2phy"
>>>>>> +    * "rockchip,rk3399-usb2phy"
>>>>>> + - #clock-cells : should be 0.
>>>>>> + - clock-output-names : specify the 480m output clock name.
>>>>>> +
>>>>>> +Optional properties:
>>>>>> + - clocks : phandle + phy specifier pair, for the input clock of phy.
>>>>>> + - clock-names : input clock name of phy, must be "phyclk".
>>>>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus.
>>>>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
>>>>> Nit: s/_/-/ here.
>>>> Something I only stumbled over yesterday for the first time on my rk3288-
>>>> popmetal: The phy subnodes seem to be able to use a generic phy-supply
>>>> property from inside the phy-core itself, see:
>>>>
>>>> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597
>>>>
>>>>
>>>> for my WIP code for that other board.
>>>>
>>> Ah, good comments! I will try later, if it is practicable, I shall correct it
>>> into the next patches (patch v4).
>>>
>> I am sorry to tell you that seems unworkable, because we have two sub-nodes
>> (phy-ports) in one parent-node (phy),
>> what is more, the 'phy-supply' property can only put into parent-node, I
>> believe it can not be differentiated types of ports.
> 'phy-supply' is a property of the phy node and not the 'phy-provider' node. So
> IMO this should work. What problem do you see?

Sorry, I think I must have made a wrong phandle of phy-port name in 
*.dts before.
Yes, It works now, thanks for your reminding.

@Heiko,
I have just received your another mail, thanks again for your detail 
explanation.

I will correct it in the next patches (patch v4).

BR.
Frank

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
new file mode 100644
index 0000000..0b4bbbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
@@ -0,0 +1,60 @@ 
+ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
+
+Required properties (phy (parent) node):
+ - compatible : should be one of the listed compatibles:
+	* "rockchip,rk3366-usb2phy"
+	* "rockchip,rk3399-usb2phy"
+ - #clock-cells : should be 0.
+ - clock-output-names : specify the 480m output clock name.
+
+Optional properties:
+ - clocks : phandle + phy specifier pair, for the input clock of phy.
+ - clock-names : input clock name of phy, must be "phyclk".
+ - vbus_host-supply : phandle to a regulator that supplies host vbus.
+ - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
+
+Required nodes : a sub-node is required for each port the phy provides.
+		 The sub-node name is used to identify host or otg port,
+		 and shall be the following entries:
+	* "otg-port" : the name of otg port.
+	* "host-port" : the name of host port.
+
+Required properties (port (child) node):
+ - #phy-cells : must be 0. See ./phy-bindings.txt for details.
+ - interrupts : specify an interrupt for each entry in interrupt-names.
+ - interrupt-names : a list which shall be the following entries:
+	* "otg_id" : for the otg id interrupt.
+	* "otg_bvalid" : for the otg vbus interrupt.
+	* "linestate" : for the host/otg linestate interrupt.
+
+Example:
+
+grf: syscon@ff770000 {
+	compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+...
+
+	u2phy: usb2-phy {
+		compatible = "rockchip,rk3366-usb2phy";
+		#clock-cells = <0>;
+		clock-output-names = "sclk_otgphy0_480m";
+
+		u2phy_otg: otg-port {
+			#phy-cells = <0>;
+			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "otg_id", "otg_bvalid", "linestate";
+			status = "okay";
+		};
+
+		u2phy_host: host-port {
+			#phy-cells = <0>;
+			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "linestate";
+			status = "okay";
+		};
+	};
+};