| Message ID | 20220606202220.1670714-2-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: dp83867: add binding and support for io_impedance_ctrl nvmem cell | expand |
> There is no documented mapping from the 32 possible values of the > IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms There have been a few active TI engineers submitting patches to TI PHY drivers. Please could you reach out to them and ask if they can provide documentation. Having magic values in DT is not the preferred why to use it. Ideally you should store Ohms in the cell and convert to the register value. Andrew
On 06/06/2022 23.58, Andrew Lunn wrote: >> There is no documented mapping from the 32 possible values of the >> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms > > There have been a few active TI engineers submitting patches to TI PHY > drivers. Please could you reach out to them and ask if they can > provide documentation. > > Having magic values in DT is not the preferred why to use it. Ideally > you should store Ohms in the cell and convert to the register value. We've already asked TI for more detailed information, but apparently the data sheet already says all there is to know. I should have worded the commit message differently. Something like There is no fixed mapping from register values to values in the range 35-70 ohms; it varies from chip to chip, and even that target range is approximate. So AFAICS the only meaningful thing to store in an nvmem cell is an appropriate (per-board) raw value of that field. I would think this would be very similar to how various sensors have nvmem cells defining calibration data. Rasmus
On Mon, 06 Jun 2022 22:22:18 +0200, Rasmus Villemoes wrote: > We have a board where measurements indicate that the current three > options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset > value or using one of the two boolean properties to set it to the > min/max value - are too coarse. > > There is no documented mapping from the 32 possible values of the > IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the > exact mapping is likely to vary from chip to chip. So add a DT binding > for an nvmem cell which can be populated during production with a > value suitable for each specific board. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > .../devicetree/bindings/net/ti,dp83867.yaml | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
On Tue, 7 Jun 2022 13:54:30 +0200 Rasmus Villemoes wrote: > On 06/06/2022 23.58, Andrew Lunn wrote: > >> There is no documented mapping from the 32 possible values of the > >> IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms > > > > There have been a few active TI engineers submitting patches to TI PHY > > drivers. Please could you reach out to them and ask if they can > > provide documentation. > > > > Having magic values in DT is not the preferred why to use it. Ideally > > you should store Ohms in the cell and convert to the register value. > > We've already asked TI for more detailed information, but apparently the > data sheet already says all there is to know. I should have worded the > commit message differently. Something like > > There is no fixed mapping from register values to values in the range > 35-70 ohms; it varies from chip to chip, and even that target range is > approximate. The series was waiting for Andrew to come back but it ended up getting marked as Changes Requested in PW. Would you mind reposting with the modification to the commit message?
On Mon, Jun 06, 2022 at 10:22:18PM +0200, Rasmus Villemoes wrote: > We have a board where measurements indicate that the current three > options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset > value or using one of the two boolean properties to set it to the > min/max value - are too coarse. > > There is no documented mapping from the 32 possible values of the > IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the > exact mapping is likely to vary from chip to chip. So add a DT binding > for an nvmem cell which can be populated during production with a > value suitable for each specific board. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml index 047d757e8d82..76ff08a477ba 100644 --- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml +++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml @@ -31,6 +31,16 @@ properties: reg: maxItems: 1 + nvmem-cells: + maxItems: 1 + description: + Nvmem data cell containing the value to write to the + IO_IMPEDANCE_CTRL field of the IO_MUX_CFG register. + + nvmem-cell-names: + items: + - const: io_impedance_ctrl + ti,min-output-impedance: type: boolean description: | @@ -42,9 +52,11 @@ properties: description: | MAC Interface Impedance control to set the programmable output impedance to a maximum value (70 ohms). - Note: ti,min-output-impedance and ti,max-output-impedance are mutually - exclusive. When both properties are present ti,max-output-impedance - takes precedence. + Note: Specifying an io_impedance_ctrl nvmem cell or one of the + ti,min-output-impedance, ti,max-output-impedance properties + are mutually exclusive. If more than one is present, an nvmem + cell takes precedence over ti,max-output-impedance, which in + turn takes precedence over ti,min-output-impedance. tx-fifo-depth: $ref: /schemas/types.yaml#/definitions/uint32
We have a board where measurements indicate that the current three options - leaving IO_IMPEDANCE_CTRL at the (factory calibrated) reset value or using one of the two boolean properties to set it to the min/max value - are too coarse. There is no documented mapping from the 32 possible values of the IO_IMPEDANCE_CTRL field to values in the range 35-70 ohms, and the exact mapping is likely to vary from chip to chip. So add a DT binding for an nvmem cell which can be populated during production with a value suitable for each specific board. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- .../devicetree/bindings/net/ti,dp83867.yaml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)