linux-kernel - Re: [PATCH v3 1/6] phy: add a driver for the Berlin SATA PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <537462A2.5020607@ti.com>
Date:	Thu, 15 May 2014 12:15:54 +0530
From:	Kishon Vijay Abraham I <kishon@...com>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Arnd Bergmann <arnd@...db.de>
CC:	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>,
	<linux-arm-kernel@...ts.infradead.org>,
	<thomas.petazzoni@...e-electrons.com>, <zmxu@...vell.com>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-ide@...r.kernel.org>,
	<alexandre.belloni@...e-electrons.com>, <jszhang@...vell.com>,
	<tj@...nel.org>
Subject: Re: [PATCH v3 1/6] phy: add a driver for the Berlin SATA PHY

Hi,

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:
>>>
>>> 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@...0ff {
>>> 	compatible = "marvell,berlin-lvds-phy";
>>> 	reg = <0xea00ff 0x100>;
>>> 	#phy-cells = <2>;
>>> };
>>>
>>> sata: sata@...0ff {
>>> 	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@...1ff {
>>> 	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.

Thanks
Kishon
--
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/