On 05/15/2014 08:45 AM, Kishon Vijay Abraham I wrote: > On Thursday 15 May 2014 12:12 AM, Sebastian Hesselbarth wrote: >> On 05/14/2014 08:12 PM, Arnd Bergmann wrote: >>> On Wednesday 14 May 2014 19:57:46 Sebastian Hesselbarth wrote: >>>> On 05/14/2014 06:57 PM, Antoine Ténart wrote: >>>>> On Wed, May 14, 2014 at 06:11:24PM +0200, Arnd Bergmann wrote: >>>>>> On Wednesday 14 May 2014 17:49:29 Antoine Ténart wrote: >>>>>>> On Wed, May 14, 2014 at 05:31:24PM +0200, Arnd Bergmann wrote: [...] >>>> Now, thinking about the PHY binding and the (possible) multi-protocol >>>> support, it can be possible that on BG2Q there is a generic 2-lane >>>> LVDS PHY that can be configured to support SATA or PCIe. Both are >>>> electrically and bit-level compatible, so they could be internally >>>> wired-up with AHCI and PCIe controller. >>> >>> Sounds like a reasonable guess. We have other PHY drivers doing the >>> same thing already. [...] >>>> From a DT point-of-view, we need a way to (a) link each SATA or PCIe >>>> port to the PHY, (b) specify the PHY lane to be used, and (c) specify >>>> the protocol to be used on that lane. If I got it right, Arnd already >>>> mentioned to use the phy-specifier to deal with it: >>>> >>>> e.g. phy = <&genphy 0 MODE_SATA> or phy = <&genphy 1 MODE_PCIE> >>> >>> Right. >>> >>>> Let's assume we have one dual-port SATA controller and one PCIe >>>> controller with either x1 or x2 support. The only sane DT binding, >>>> I can think of then would be: >>>> >>>> berlin2q.dtsi: >>>> >>>> genphy: lvds@ea00ff { >>>> compatible = "marvell,berlin-lvds-phy"; >>>> reg = <0xea00ff 0x100>; >>>> #phy-cells = <2>; >>>> }; >>>> >>>> sata: sata@ab00ff { >>>> compatible = "ahci-platform"; >>>> reg = <0xab00ff 0x100>; >>>> >>>> sata0: sata-port@0 { >>>> reg = <0>; >>>> phy = <&genphy 0 MODE_SATA>; >>>> status = "disabled"; >>>> }; >>>> >>>> sata1: sata-port@1 { >>>> reg = <1>; >>>> phy = <&genphy 1 MODE_SATA>; >>>> status = "disabled"; >>>> }; >>>> }; >>>> >>>> pcie: pcie@ab01ff { >>>> compatible = "marvell,berlin-pcie"; >>>> reg = <0xab01ff 0x100>; >>>> >>>> pcie0: pcie-port@0 { >>>> reg = <0>; >>>> /* set phy on a per-board basis */ >>>> /* PCIe x1 on Lane 0 : phy = <&genphy 0 MODE_PCIE>; */ >>>> /* PCIe x2 on Lane 0 and 1 : phy = <&genphy 0 MODE_PCIE>, <&genphy 1 >>>> MODE_PCIE>; */ >>>> status = "disabled"; >>>> }; >>>> }; >>>> >>>> berlin2q-dmp.dts: >>>> >>>> &sata1 { >>>> status = "okay"; >>>> }; >>>> >>>> &pcie0 { >>>> phy = <&genphy 1 MODE_PCIE>; >>>> }; >>>> >>>> berlin2q-foo.dts: >>>> >>>> &pcie0 { >>>> phy = <&genphy 0 MODE_PCIE>, <&genphy 1 MODE_PCIE>; >>>> }; >>> >>> Exactly. I would also be fine with keeping the sub-nodes of the >>> phy device as in v3 and using #phy-cells=<1> instead of #phy-cells. >>> The result would be pretty much the same, it just depends on how >>> closely connected the two logical phys are. > > huh.. even with sub-nodes you'll need #phy-cells=<2> if we use a single *PHY > PROVIDER*. Because with just PHYs node pointer we won't be able to get the PHY. > We'll need PHY providers node pointer. > > However I'd prefer to have sub-nodes for each individual PHYs and register a > single PHY PROVIDER. Depends on what you call PHY. In the example above the PHY is what allows you to control both lanes. So you want sub-nodes for each individual lane given the nomenclature of the example? Or like it is used in the example above, a single PHY node with an index in the phy-specifier to pick an individual lane. IMHO, having both phy-specifier index _and_ PHY sub-node per lane has no benefit at all. You cannot even use the PHY sub-nodes for any setup properties, as they depend on the consumer claiming the lane. Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html