lkml.org 
[lkml]   [2016]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [net-next PATCH 1/3] net: phy: dp83867: Add documentation for optional impedance control
From
Date
Rob,

On Friday 22 July 2016 02:44 AM, Rob Herring wrote:
> On Thu, Jul 21, 2016 at 11:52:36AM +0530, Sekhar Nori wrote:
>> Nishanth,
>>
>> On Wednesday 20 July 2016 09:03 PM, Nishanth Menon wrote:
>>> On 07/20/2016 09:56 AM, Mugunthan V N wrote:
>>>> Add documention of ti,impedance-control which can be used to
>>>> correct MAC impedance mismatch using phy extended registers.
>>>>
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> ---
>>>> Documentation/devicetree/bindings/net/ti,dp83867.txt | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt
>>>> b/Documentation/devicetree/bindings/net/ti,dp83867.txt
>>>> index 5d21141..531ea3c5 100644
>>>> --- a/Documentation/devicetree/bindings/net/ti,dp83867.txt
>>>> +++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
>>>> @@ -9,6 +9,13 @@ Required properties:
>>>> - ti,fifo-depth - Transmitt FIFO depth- see
>>>> dt-bindings/net/ti-dp83867.h
>>>> for applicable values
>>>>
>>>> +Optional property:
>>>> + - ti,impedance-control - MAC Interface Impedance control to vary the
>>>> + output impedance with an approximate range
>>>> + from 35-70 ohms in 32 steps. Value range can
>>>> + be 0x0 to 0x1f. Lowest impedance value is
>>>> + 0x1f and highest impedance being 0x0.
>>>> +
>>>
>>> Should'nt you be using the impedance values of 35 to 70 as the valid
>>> values here? that would be the hardware description, and the values to
>>> program corresponding to those are 0x00 to 0x1f. Right?
>>>
>>> Rob: is'nt that the right way to do it?
>
> Normally, yes that is preferred. I don't have an issue with it here if
> that is what makes sense.
>
>> Agree that that is usually the right way to do it. I believe this case
>> is bit peculiar though. Here is the extract from the datasheet[1] about
>> how the register in question works.
>>
>> "
>> Output impedance approximate range from 35-70 ohms in 32 steps.
>> Lowest being 11110 and highest being 00000. Range and Step size
>> will vary with process.
>>
>> Default is set to 50 ohms by trim. But the default register value can
>> vary by process. Non default values of MAC I/O impedance can be
>> used based on trace impedance. Mismatch between device and
>> trace impedance can cause voltage overshoot and undershoot.
>> "
>>
>> So clearly, there is no easy correspondence that the hardware guarantees
>> between output impedance ohmage and the register value programmed. Only
>> couple of things are guaranteed.
>>
>> 1) Programming a value of 0 gives approximately 35 ohms
>> 2) Programming a value of 0x1F gives approximately 70 ohms
>> 3) Default value of the register gives 50 ohms (the default value could
>> vary by device).
>> 4) Programming a value in between will give some ohmage in the
>> approximate range 35-70 (curve is unknown).

Sekhar got it wrong, programming 0x1F provides the minimum impedance of
35 ohms and 0x0 provides the maximum impedance of 70 ohms.

>>
>> Given this, I am not sure how one can convert a given user supplied ohms
>> values to a reasonable register value. Clearly, the register is supposed
>> to be programmed by experimentation, not calculation.
>
> So your are going to change from a known value tuned for the individual
> Si (50ohm) to the same value for all chips (at least for a given board).
> Or does this get set by firmware at boot? If not, seems like you would
> want to specify a delta from the default register value in this case.

The delta will not be constant for each steps of the register content.
So this approach cannot be taken.

>
>> That said, we could take another approach. At least for the current
>> issue we are trying to address, we only need to configure the register
>> to max value. And given the nature of the register, I am pretty sure
>> that is what most people will end up doing.
>
> How does the max value get determined? Possibly it should just be set in
> the kernel.

Yes, the min/max value will be determined in the kernel and the DT will
have either of the properties below.

>
>> So, may be we have two properties:
>>
>> ti,max-output-impedance
>> ti,min-output-imepdance
>
> Just define max if that's all you currently need.

Currently we care about only the min-output-impedance (register content
0x1f). Will respin the patch using ti,min-output-impedance.

Regards
Mugunthan V N

\
 
 \ /
  Last update: 2016-07-22 12:41    [W:0.112 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site