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

[Kishon Vijay Abraham I <kishon@ti.com>]

On Thursday 15 May 2014 02:47 PM, Sebastian Hesselbarth wrote:
> On 05/15/2014 10:46 AM, Kishon Vijay Abraham I wrote:
>> On Thursday 15 May 2014 12:32 PM, Sebastian Hesselbarth wrote:
>>> 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:
>>>>>>> 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>;
>>>>>>> };
> [...]
>>>
>>> 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.
>>
>> IMO the dt data should completely describe the HW. So just by looking at the
>> PHY node, we won't be able to tell the no of PHYs implemented in the IP if we
>> have a single PHY node (In this case the lanes in the IP).
>>
>> However if you think it's an overkill for having sub-nodes for each lane then
>> single PHY node is fine too.
> 
> Yeah, I see your point. I just wonder how many Marvell PHYs we may hit
> that require the _same_ magic setup inside but have _different_ number
> of lanes. And even if, we can deal with it using a different compatible
> string.
> 
> Currently, I feel a single PHY provider node and a set of compatibles
> will be most likely, i.e. no per-lane sub-nodes. OTOH, the per-lane
> sub-nodes is more generic as it allows us to deal PHYs that may
> suddenly skip one lane in the numbering scheme. The difference for the
> driver is marginal, i.e. some SoC-specific struct with a field for the
> number of lanes vs. of_count_child_nodes() and a reg = <n> property for
> the per-lane sub-nodes.
> 
> I used to agree to "DT should descibe HW", but with no datasheet
> available, it quickly becomes fuzzy what it really looks like.
> 
> Anyway, I'll have some discussion with Antoine and Alexandre to sort out
> the differences and the things in common for the PHY and SoCs in
> question.

cool, thanks.

-Kishon