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: >> >> From what I understand from the conversation, we have a single PHY >> register set dealing with both SATA ports available on the SoC. >> Also, from the name of the PHY bits we assume the PHY may be able >> to work in different modes than just SATA. And we currently have >> an AHCI-compatible SATA IP that supports up to two ports, with one >> actually connected to a SATA plug on the DMP board. >> >> 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. Well, I based that on what I know about FPGA LVDS transceivers, so I wasn't guessing out of the blue ;) >> 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. > > IIRC for the ST microelectronics PHY we recently reviewed, the > same PHY could be driving multiple SATA ports, or a single > multi-lane PCIe. In that case, it makes more sense to have > #phy-cells=<2>, like your example. I guess the final call is up to Kishon, but I personally prefer #phy-cells = <2>. It makes no functional difference though. >> For the driver, Antoine then would have to squeeze all PHY register >> mangling in phy-berlin2.c and see how to make ahci-platform aware of >> individual port nodes (I haven't looked up if it already exists, sorry) >> and announce only enabled port child nodes, right? > > I've been thinking some more about this aspect. I don't actually have > a strong opinion on whether it's better to use the generic ahci-platform > driver, or to keep the multi-phy support as a special variant for > berlin. If we do the latter, it would however be good to define the > binding in a way that lets us later merge things into the generic phy > driver in case we get more of the same. Hmm, IMHO multi-phy support is orthogonal to ahci-platform, isn't it? ahci-platform needs to know about the phy property and calls some helper that deals with the phy-specifier? About a generic _phy_ driver, I am not so sure if berlin is the best template right now ;) So, my call would be: - make ahci-platform aware of port sub-nodes and phy properties - have a berlin specific PHY driver 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