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@xxxxxx> >>>> --- >>>> 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 -- 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